-
Notifications
You must be signed in to change notification settings - Fork 189
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(datafusion): Support pushdown more datafusion exprs to Iceberg #649
Conversation
@liurenjie1024 @Xuanwo @sdd @a-agmon please help to review when you are free, thanks |
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.
TransformedResult::Predicate(Predicate::Binary(BinaryExpression::new(op, r, d))) | ||
} | ||
|
||
fn reserve_predicate_operator(op: PredicateOperator) -> PredicateOperator { |
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.
I think you meant reverse in the function name rather than reserve?
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.
yes, thanks for pointing, updated
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.
Hi @FANNG1 - this looks like a really elegant solution to a tricky problem. Thanks for the PR! Approved :-)
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.
…pache#649) * suppport more datafusion predict * add it * reverse op
fixes: #644
DataFusion expressions and Iceberg predicates do not have a one-to-one correspondence.
Expr
in DataFusion, whereas they are merely components of predicates in Iceberg.And
andOr
are operators in DataFusion, but they are treated as predicates in Iceberg.This disparity complicates the transformation process when using
TreeNode
. To address this, I also refactored the code to simplify it.