Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add visitor for converting kernel expressions to engine expressions #363

Merged
Merged
Show file tree
Hide file tree
Changes from 83 commits
Commits
Show all changes
87 commits
Select commit Hold shift + click to select a range
6e1bbf1
Move ffi code, add kernel expr -> engine expr
OussamaSaoudi-db Sep 27, 2024
b367df1
Remove new visitor
OussamaSaoudi-db Sep 27, 2024
1bc700a
Add kernel expression to engine expression
OussamaSaoudi-db Sep 27, 2024
dc1ed23
Add ffi validation and more visitor features
OussamaSaoudi-db Sep 30, 2024
8ce6338
initial type expansion
OussamaSaoudi-db Oct 1, 2024
384056b
Add new impls for visitor
OussamaSaoudi-db Oct 1, 2024
10b6534
Add even more expression visitor stuff
OussamaSaoudi-db Oct 1, 2024
e7aa687
Patch expr_struct
OussamaSaoudi-db Oct 2, 2024
57fdf77
Fix ffi test
OussamaSaoudi-db Oct 2, 2024
27736a0
Fix clippy issues
OussamaSaoudi-db Oct 2, 2024
5ff4396
Add support for structs, arrays, null
OussamaSaoudi-db Oct 3, 2024
bacc7b2
Improve literal type system
OussamaSaoudi-db Oct 4, 2024
318453e
Remove cache files
OussamaSaoudi-db Oct 7, 2024
5217dc6
Add Remaining expression visitor functionality
OussamaSaoudi-db Oct 7, 2024
778c84c
Add deconstructor and make functions consistently named
OussamaSaoudi-db Oct 7, 2024
07c458b
More fixes to expression
OussamaSaoudi-db Oct 9, 2024
23d1ed2
Fix all memory leaks
OussamaSaoudi-db Oct 9, 2024
ccf47b2
Merge branch 'main' into expresions_visitor2
OussamaSaoudi-db Oct 9, 2024
c66fb3e
Remove clang cache
OussamaSaoudi-db Oct 9, 2024
45c2a99
Fix safety, remove unnecessary diffs
OussamaSaoudi-db Oct 9, 2024
097fe8b
Add docs to visit_expression
OussamaSaoudi-db Oct 10, 2024
4b0ab65
Add expressions test
OussamaSaoudi-db Oct 10, 2024
1af2939
Add docs, improve style
OussamaSaoudi-db Oct 10, 2024
4df19e3
Fix spacing
OussamaSaoudi-db Oct 10, 2024
283594b
Fix diff, move test to beginning of read_table
OussamaSaoudi-db Oct 10, 2024
57271fb
Fix cross-platform printing
OussamaSaoudi-db Oct 10, 2024
71cdf3b
Merge branch 'main' into expresions_visitor2
OussamaSaoudi-db Oct 10, 2024
a24419d
Improve C error handling, remove kernel expression from main
OussamaSaoudi-db Oct 10, 2024
11fc68a
Remove changes from read_table.c
OussamaSaoudi-db Oct 10, 2024
c3160ad
remove unnecessary changes
OussamaSaoudi-db Oct 10, 2024
295e8e8
Move testing code into separate executable
OussamaSaoudi-db Oct 10, 2024
b069cf1
Revamp testing
OussamaSaoudi-db Oct 11, 2024
69e92f4
Move to sibling-list_based approach
OussamaSaoudi-db Oct 11, 2024
c8f80a3
Fix free
OussamaSaoudi-db Oct 11, 2024
006adfb
fix visit_struct naming
OussamaSaoudi-db Oct 11, 2024
30e0596
Fix formtting and style in test expr
OussamaSaoudi-db Oct 11, 2024
aef2928
Remove KernelStringSlice from c side code
OussamaSaoudi-db Oct 11, 2024
3cfeb2a
Move to using allocate_string
OussamaSaoudi-db Oct 11, 2024
f2d4cf4
Remove read_table dependency in expresison
OussamaSaoudi-db Oct 11, 2024
15e52eb
Fix tests, change enum naming to be clearer
OussamaSaoudi-db Oct 11, 2024
595280f
Use system header for assert
OussamaSaoudi-db Oct 11, 2024
7e7c847
rename literals
OussamaSaoudi-db Oct 11, 2024
a6ddd40
fix memory leaks
OussamaSaoudi-db Oct 11, 2024
24dbce4
Revert read_table
OussamaSaoudi-db Oct 11, 2024
9019da1
Improve naming
OussamaSaoudi-db Oct 11, 2024
406cfe7
Put in documentation
OussamaSaoudi-db Oct 12, 2024
cfb1ed7
Change arg order
OussamaSaoudi-db Oct 12, 2024
fe339d1
Remove unnecessary change
OussamaSaoudi-db Oct 12, 2024
ba3153a
Fix grammar
OussamaSaoudi-db Oct 12, 2024
1b8575c
Move test function to separate module
OussamaSaoudi-db Oct 14, 2024
6cfe39f
Add feature flag to test
OussamaSaoudi-db Oct 14, 2024
bd2332c
Make sibling id second arg always
OussamaSaoudi-db Oct 14, 2024
046147d
Test binary data
OussamaSaoudi-db Oct 14, 2024
a2d0dbf
move abort to assert
OussamaSaoudi-db Oct 14, 2024
d46b4e3
Small cosmetic changes
OussamaSaoudi-db Oct 14, 2024
75a84a3
Address PR comments
OussamaSaoudi-db Oct 15, 2024
d8250ac
Remove unneccessary changes
OussamaSaoudi-db Oct 15, 2024
748c45a
better naming
OussamaSaoudi-db Oct 15, 2024
8f1b2eb
Improve doc comment
OussamaSaoudi-db Oct 15, 2024
d67d7d1
Spacing
OussamaSaoudi-db Oct 15, 2024
05e95dc
Remove leaks test
OussamaSaoudi-db Oct 15, 2024
22b8896
Rename print_tree, take out implementation detail of lists from examples
OussamaSaoudi-db Oct 15, 2024
539d9c6
Address PR comments
OussamaSaoudi-db Oct 15, 2024
456d727
More pr addressing
OussamaSaoudi-db Oct 15, 2024
5c1a2f8
Simplify Variadic and Unary
OussamaSaoudi-db Oct 15, 2024
ffc3d51
Fix build error
OussamaSaoudi-db Oct 15, 2024
0c88b31
fix build
OussamaSaoudi-db Oct 15, 2024
9a73536
Fix kernel issue
OussamaSaoudi-db Oct 15, 2024
3683a85
Follow PR recommendations
OussamaSaoudi-db Oct 16, 2024
3f37450
Fix typo
OussamaSaoudi-db Oct 18, 2024
6abdd78
Merge branch 'main' into expresions_visitor2
OussamaSaoudi-db Oct 22, 2024
03c1440
Fix rebase issue
OussamaSaoudi-db Oct 23, 2024
bdd28d7
Rename the expressions modules
OussamaSaoudi-db Oct 24, 2024
14a5c15
Apply review comments
OussamaSaoudi-db Oct 24, 2024
c22e311
Address comments
OussamaSaoudi-db Oct 24, 2024
49f6198
Try windows without MSVC
OussamaSaoudi-db Oct 24, 2024
633a4ba
Fix wording
OussamaSaoudi-db Oct 24, 2024
4faff29
Add dev visibility
OussamaSaoudi-db Oct 24, 2024
984b615
Flushing changes
OussamaSaoudi-db Oct 24, 2024
8530b8d
Address pr comments
OussamaSaoudi-db Oct 24, 2024
306c982
Merge branch 'main' into expresions_visitor2
OussamaSaoudi-db Oct 24, 2024
bbac1bb
Fix rebase issue
OussamaSaoudi-db Oct 24, 2024
00d0354
hopefully fix linux leak test
OussamaSaoudi-db Oct 24, 2024
d282063
Remove failing test
OussamaSaoudi-db Oct 25, 2024
d0218fd
Renaming
OussamaSaoudi-db Oct 25, 2024
60049ab
Small nit
OussamaSaoudi-db Oct 25, 2024
abcf590
Merge branch 'main' into expresions_visitor2
OussamaSaoudi-db Oct 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ jobs:
sudo apt update
sudo apt install -y -V libarrow-dev # For C++
sudo apt install -y -V libarrow-glib-dev # For GLib (C)
sudo apt install -y -V valgrind # For memory leak test
elif [ "$RUNNER_OS" == "macOS" ]; then
brew install apache-arrow
brew install apache-arrow-glib
Expand All @@ -108,16 +109,24 @@ jobs:
cargo build
popd
pushd ffi
cargo b --features default-engine,sync-engine
cargo b --features default-engine,sync-engine,test-ffi
popd
- name: build and run test
- name: build and run read-table test
run: |
pushd ffi/examples/read-table
mkdir build
pushd build
cmake ..
make
make test
- name: build and run visit-expression test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this run in parallel with the first (low prio)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is sequential.

run: |
pushd ffi/examples/visit-expression
mkdir build
pushd build
cmake ..
make
make test

coverage:
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,4 @@ default-engine = [
]
sync-engine = ["delta_kernel/sync-engine"]
developer-visibility = []
test-ffi = []
36 changes: 36 additions & 0 deletions ffi/examples/visit-expression/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
cmake_minimum_required(VERSION 3.12)
project(visit_expressions)

add_executable(visit_expression visit_expression.c)
target_compile_definitions(visit_expression PUBLIC DEFINE_DEFAULT_ENGINE)
target_include_directories(visit_expression PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../../../target/ffi-headers")
target_link_directories(visit_expression PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../../../target/debug")
target_link_libraries(visit_expression PUBLIC delta_kernel_ffi)
target_compile_options(visit_expression PUBLIC)

target_compile_options(visit_expression PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-strict-prototypes)

# Get info on the OS and platform
if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zachschuermann just wanted to have a second look. Slightly yucky, but it's the way I found to target linux for valgrind. Note that macos does not support valgrind.

set(MACOSX TRUE)
endif()
if(UNIX AND NOT APPLE)
set(LINUX TRUE)
endif()

# Add the kernel expresion -> engine expression test
include(CTest)
set(ExprTestRunner "../../../tests/test_expression_visitor/run_test.sh")
set(ExprExpectedPath "../../../tests/test_expression_visitor/expected.txt")
add_test(NAME test_expression_visitor COMMAND ${ExprTestRunner} ${ExprExpectedPath})

if(LINUX)
add_test(NAME test_expression_visitor_leaks COMMAND valgrind
--error-exitcode=1
--tool=memcheck
--leak-check=full
--errors-for-leak-kinds=definite
--show-leak-kinds=definite ./visit_expression)
elseif(MACOSX)
add_test(NAME test_expression_visitor_leaks COMMAND leaks --atExit -- ./visit_expression)
endif()
Loading
Loading