-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Implement PullUp Empty Results optimizer #13524
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
Implement PullUp Empty Results optimizer #13524
Conversation
Main needs to be merged into feature, then the commit history should be cleaned up a little bit |
…s refer to the CTE, and if it is removed, then there are heap use after free errors
8821d57
to
8f5f4cb
Compare
Can you fix the conflict please? |
…ith_empty_result_if_child_is_empty
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.
Thanks for the PR! This looks good, I just have some minor comments:
|
||
namespace duckdb { | ||
|
||
//! The Deliminator optimizer traverses the logical operator tree and removes any redundant DelimGets/DelimJoins |
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.
Could you update this comment to describe this operator?
} | ||
|
||
unique_ptr<LogicalOperator> Optimize(unique_ptr<LogicalOperator> op); | ||
unique_ptr<LogicalOperator> PullUpEmptyJoinChildren(unique_ptr<LogicalOperator> op); |
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.
Could you make PullUpEmptyJoinChildren
private?
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.
Thanks for the changes, LGTM!
Thanks! Looks great |
Currently this logic is sprinkled within filter pull up and filter push down. The problem is it's not at every operator, and adding empty results across multiple files within the filter pullup/pushdown operator can make the code hard to maintain and debug.
This will unify the code and fix some fuzzer issues that are the cause of long execution times on the LHS of a join while a child on the RHS is empty . It can also help make other queries faster in a few cases.
One con is that some
ConversionErrors
will be hidden.