Repository navigation
Conversation
… sets using a sorted value set Since apacheGH-46777 is_in predicates with more than 50 values are not simplified against row group statistics at all, because filtering the value set and rebinding the call for every guarantee was prohibitively expensive. Large value sets are now simplified with a binary search instead: - The is_in kernel init keeps a copy of the value set sorted in the order of the comparison kernels, with nulls removed. An already sorted value set is reused as is after a single O(N) check; an unsorted one is sorted lazily and thread-safely on the first simplification, so eager kernel calls never pay for sorting. Value sets containing NaN are not searchable, since statistics cannot bound NaN. - SimplifyWithGuarantee applies all inequality guarantees on a field to a large is_in call at once, narrowing an index range into the sorted value set with lower/upper bound searches. If the range is empty the call simplifies to false (subject to the null matching behavior), otherwise it is left unchanged. This is exact for any number of values, costs O(log N) per guarantee and never rebinds. Applying both bounds together is what allows pruning a row group inside the hull of the value set but holding none of its values, which no per-bound check can do. Value sets up to 50 values keep using the existing filter-and-rebind path.
…d of via sort_indices + take Sorting a copy of the values with std::sort is about 14x faster than the generic sort_indices + take path for a shuffled set of 10M int64 values (0.19s vs 2.6s). Nulls are dropped while copying and a set containing NaN is reported as not searchable before sorting, since NaN would break the strict weak ordering std::sort requires. Non-primitive types (binary, decimal) keep using the generic path.
|
|
github-actions
Bot
added
Component: C++
awaiting committer review
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
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.
Rationale for this change
Since #46859,
is_inpredicates with more than 50 values are not simplified against Parquet row group statistics at all, so such filters read every row group. See #55292 for the background. The same problem was reported and fixed in Polars in pola-rs/polars#29923.What changes are included in this PR?
is_inkernel state keeps a sorted, null-free copy of the value set when it has more than 50 values. An already sorted set is reused as is after a single O(N) check; otherwise it is sorted lazily on first use, so plain kernel calls don't pay for it. Primitive sets are sorted withstd::sorton a copy, which is about 13x faster thansort_indices+takefor 10M shuffled values.SimplifyWithGuaranteeapplies all inequality guarantees on a field to a largeis_incall at once by binary searching the sorted set. If no value can match, the call becomesfalse, otherwise it is left alone (no rebinding). Using both row group bounds together is what lets us skip row groups that lie inside the hull of the value set but hold none of its values.Comparison::StripOrderPreservingCastsno longer strips unsigned to signed casts that may wrap. This also fixes the existing comparison simplifier.Are these changes tested?
New
Expression.SimplifyIsInLargeValueSetandExpression.SimplifyLargeIsInThenExecutetests, plus a Parquet dataset test that prunes row groups inside the hull of the value set.Scan timings on a 49M row file with 47 row groups (release build, file in page cache): filters with 100 to 100K ids go from ~100 ms (the whole file) to 4 to 8 ms, 1M ids from ~300 ms to 26 to 42 ms, 10M ids from ~450 ms to ~260 ms. A value set that touches every row group costs the same as before.
Are there any user-facing changes?
No API changes.
is_infilters with more than 50 values prune Parquet row groups again.Was AI used for this PR?
PR code and description written by:
Reviewed before submission by:
The code was drafted with an AI assistant; I reviewed, tested and benchmarked it and iterated on the design.