-
Notifications
You must be signed in to change notification settings - Fork 915
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
Abstract Syntax Tree Cleanup and Tests #7418
Abstract Syntax Tree Cleanup and Tests #7418
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at all this, I think it could be further improved by making the ast_plan
own both the host_data_buffer
and device_data_buffer
as well as provide member functions for accessing the aliased pointers into the device data buffer in order to move that messy logic inside the class.
Yea, I realized there was more logic/work that could be moved to the ctor (as pointed out by the linked comment in the issue that I clicked on but GitHub hung for a while retrieving it - so I went and worked on something and forgot to go back and check). Will address. |
Codecov Report
@@ Coverage Diff @@
## branch-0.20 #7418 +/- ##
===============================================
+ Coverage 82.88% 82.92% +0.03%
===============================================
Files 103 104 +1
Lines 17668 17901 +233
===============================================
+ Hits 14645 14845 +200
- Misses 3023 3056 +33
Continue to review full report at Codecov.
|
This PR has been labeled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small stuff.
@gpucibot merge |
This PR implements conditional joins using expressions that are decomposed into abstract syntax trees for evaluation. This PR builds on the AST evaluation framework established in #5494 and #7418, but significantly refactors the internals and generalizes them to enable 1) expressions on two tables and 2) operations on nullable columns. This PR uses the nested loop join code created in #5397 for inner joins, but also substantially generalizes that code to enable 1) all types of joins, 2) joins with arbitrary AST expressions rather than just equality, and 3) handling of null values (with user-specified `null_equality`). A significant chunk of the code is currently out of place, but since this changeset is rather large I've opted not to move things in ways that will make reviewing this PR significantly more challenging. I will make a follow-up to address those issues once this PR is merged. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Jake Hemstad (https://github.com/jrhemstad) - Conor Hoekstra (https://github.com/codereport) URL: #8214
Resolves #6320