adaptive filters should not reorder filters that can throw #14672
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tmonster
commented
Nov 4, 2024
There was a problem hiding this comment.
I changed the types on within the parquet file because otherwise the test would fail. I think the test is more to test union by name, and not to test the types and conversions, so I thought changing the types so the query works in all cases would be best
|
Thanks! |
github-actions bot
pushed a commit
to duckdb/duckdb-r
that referenced
this pull request
Dec 21, 2024
adaptive filters should not reorder filters that can throw (duckdb/duckdb#14672)
github-actions bot
added a commit
to duckdb/duckdb-r
that referenced
this pull request
Dec 21, 2024
adaptive filters should not reorder filters that can throw (duckdb/duckdb#14672) Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
Mytherin
added a commit
that referenced
this pull request
May 8, 2025
This pr tries to fix #16552 with just one simple idea: JoinCondition may be faster, so it should be executed first instead of arbitrary expressions. And I have some doubts when checking this issue: Does the order of filter expressions in sql have to be respected if any expression ``CanThrow() == true``? If not, then ``ExpressionHeuristics`` shouldn't skip optimising filter expression sequence as modified in #14672 If indeed it needs to do so, then ``LogicalComparisonJoin::ExtractJoinConditions`` shouldn't split expressions by ``LogicalFilter::SplitPredicates(expressions)`` because this method doesn't keep original expressions sequence.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes https://github.com/duckdblabs/duckdb-internal/issues/3180
And this is a followup to #13617
If the table being filtered is large enough, the order of the expressions will get moved around due to adaptive filtering. To prevent this, expression reordering now places severe penalties on expressions that can throw, effectively forcing them to be executed last. Then during adaptive filters, we set the likelihood of swapping filters that can throw to 0. This allows other filters to be swapped, but not filters. that can throw.