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

feat: add kernel ExpressionEvaluator #1829

Merged
merged 9 commits into from
Dec 11, 2023
Merged

feat: add kernel ExpressionEvaluator #1829

merged 9 commits into from
Dec 11, 2023

Conversation

roeap
Copy link
Collaborator

@roeap roeap commented Nov 9, 2023

based on #1807

Description

In the effort to advance protocol support and move our internal APIs closer to the kernel library, it is advantageous to leverage the expression handling logic from kernel specifically for filtering actions etc.

This PR just add the expression definitions and evaluation logic. Integrating it with our current codebase and basing the existing partition handling logic on this is left for follow up PRs to keep thigs review-able.

related: #1894, #1776

Copy link

github-actions bot commented Nov 9, 2023

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@roeap roeap changed the title [WIP] chore: add kernel ExpressionHandler chore: add kernel ExpressionEvaluator Nov 9, 2023
@roeap roeap force-pushed the expressions branch 2 times, most recently from fab4a26 to b49170b Compare November 10, 2023 07:54
@github-actions github-actions bot added binding/rust Issues for the Rust crate crate/core crate/sql labels Nov 10, 2023
@Blajda
Copy link
Collaborator

Blajda commented Nov 16, 2023

Once this is merged in do you suggest we remove the CNF expressions we use for optimize? Seem like we should depreciate that interface and also create a bridge between string expressions and maybe extra code to convert to an equivalent Datafusion expression.

@roeap
Copy link
Collaborator Author

roeap commented Nov 16, 2023

From the kernel side, the general idea is to have a way of defining expressions without additional dependencies. So yes, we should ten likely define conversions to datafusion expressions. I only looked a bit into our partition filters, if we could remove those too... From the python API side at least we probably want to keep CNF expressions around?

@roeap roeap force-pushed the expressions branch 3 times, most recently from 348450c to 09b55b0 Compare November 17, 2023 06:54
@roeap roeap changed the title chore: add kernel ExpressionEvaluator feat: add kernel ExpressionEvaluator Dec 3, 2023
@roeap roeap marked this pull request as ready for review December 3, 2023 09:07
@roeap roeap requested a review from Blajda December 3, 2023 09:09
@roeap roeap enabled auto-merge (squash) December 11, 2023 17:33
@roeap roeap merged commit f69d36f into delta-io:main Dec 11, 2023
21 checks passed
@roeap roeap deleted the expressions branch December 11, 2023 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants