Repository navigation
[Data] Prune Parquet row groups for derived predicates - #67943
sainimal1ba-hue wants to merge 18 commits into
Conversation
b698c34 to
e916bcf
Compare
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
e916bcf to
92aff7f
Compare
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
5d6c105 to
8ce9704
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a _derived_statistics_predicate helper function to derive simplified predicates (such as positive integer addition and string prefix matches) that Parquet statistics can evaluate, enabling more effective row-group pruning during Parquet reads. Comprehensive unit tests are also added to verify the correctness and safety of these derived predicates. The reviewer suggested improving the robustness of the derived predicate pruning by supporting commutative addition (e.g., 1 + col("x") in addition to col("x") + 1).
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
fa5456d to
db5c790
Compare
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces _derived_statistics_predicate to optimize Parquet row-group pruning by deriving simpler predicates from arithmetic operations and string prefix matches, along with comprehensive unit tests. The feedback suggests extending this optimization to compound filters by recursively traversing logical AND and OR operators, and adding corresponding test cases to verify this behavior.
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces a _derived_statistics_predicate helper to extract simplified conditions from filter expressions (such as addition offsets and string prefixes) for Parquet row-group pruning, and integrates it into the FooterReader. The review feedback recommends dynamically determining the column's bit_width for signed integers instead of hardcoding 64-bit limits, which can cause failures or silent disabling of pruning for smaller integer types like int32. It also suggests adding corresponding test cases to verify that unsafe arithmetic is correctly skipped for these smaller types.
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit be1cf4e. Configure here.
There was a problem hiding this comment.
Code Review
This pull request introduces _derived_statistics_predicate to derive simplified predicates from complex filter expressions that Parquet statistics can evaluate, allowing for more aggressive row-group pruning during footer reading. It also adds comprehensive tests to verify this behavior under various conditions, including arithmetic operations, string prefixes, and null values. The review feedback suggests extending this optimization by preserving simple unary operations (like IS_NULL and IS_NOT_NULL) and list operations (like IN and NOT_IN) within compound predicates to further improve pruning power, which also requires importing UnaryExpr.
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to derive a predicate that Parquet statistics can evaluate (_derived_statistics_predicate) to prune row groups during reading, along with comprehensive tests. The feedback suggests optimizing the derived predicate for Operation.IN by avoiding the conservative null-inclusive bound (| expr.left.is_null()), which is only necessary for Operation.NOT_IN.
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces row-group pruning based on derived statistics predicates in the Parquet footer reader. It adds the _derived_statistics_predicate helper to extract evaluable conditions from expressions and applies them during the footer reading process to drop irrelevant row groups. Extensive unit tests are also added to cover various edge cases. The reviewer suggested catching all Exceptions instead of a specific list of exceptions when deriving the predicate to prevent any unexpected runtime errors from crashing the read operation.
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces a _derived_statistics_predicate helper to extract safe pruning predicates from filter expressions, allowing row-group pruning based on Parquet statistics. It also adds comprehensive unit tests covering various data types and edge cases. The reviewer suggests extending this optimization to support unsigned integer columns by dynamically calculating the minimum and maximum bounds based on the integer's signedness.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces a _derived_statistics_predicate helper function to extract safe, evaluable Parquet statistics predicates from filter expressions, integrating it into the FooterReader to improve row-group pruning efficiency. It also adds extensive unit tests covering various predicate types, boundary conditions, and fallback behaviors. Feedback on the changes suggests expanding the membership check to support other collection types like tuple and set to prevent unnecessary null-inclusive fallbacks, and supporting NumPy integer types in arithmetic bound derivations by safely casting them to Python int to avoid silent overflow issues.
Signed-off-by: Sai Nimal <sainimal@Sais-MacBook-Air.local>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new function _derived_statistics_predicate in the Parquet footer reader to derive safe pruning predicates from filter expressions, enabling row-group pruning for operations like integer addition and string prefix matching. It also updates FooterReader to utilize these derived predicates and adds extensive unit tests. The review feedback suggests improving the robustness of the derived predicate logic for integer addition by normalizing or mirroring comparison operands and operators to support mirrored comparisons.
|
Hi @sainimal1ba-hue, thank you for your contribution! path = "/tmp/t.parquet"
x = list(range(1000))
pq.write_table(pa.table({"x": x, "f": [float(v) for v in x]}), path, row_group_size=200)
def kept(expr):
reader = FooterReader(pafs.LocalFileSystem(), filter_expr=expr)
chunked = reader._read_and_chunk(FileInfo(path, os.path.getsize(path)))
return [c.unit_ids[0] for c in chunked.row_groups]
print(kept(((col("x") + 1) >= 1) & ~(col("x") < 800)))
print(kept(col("f").is_in([900.0])))
print(kept(~(col("f") < 800.0)))The NaN rows the float check protects are dropped anyway by pyarrow's own scanner in the read task ( |
The derived predicate replaced the original filter for row-group pruning, and a float filter without a derived bound skipped pruning entirely. Both made pruning worse than master for filters PyArrow already evaluates against statistics, e.g. float membership, negated comparisons and compound filters. Prune with the original filter first, then narrow the survivors with the derived bound, which is a necessary condition the statistics can evaluate. Filters PyArrow cannot evaluate against statistics keep their pruning through the derived bound alone, as before. PyArrow's scanner drops NaN rows for float filters regardless of what pruning does, so NaN handling stays with PyArrow. Adds a regression test for pruning parity on the reported repro.
added a regression test. all 247 tests in the file pass. dropped the float guard like u told. scanner drops those NaN rows anyway. swapped the NaN preservation test for one that checks the derived bound never contradicts a NaN row. kept master pruning. your three cases all give [4]. derived bound runs next to the original filter and only narrows more, so (x + 1) >= 76 and starts_with still prune. if the original filter cant handle a column the derived bound prunes alone. no derived bound, same as master. |
|
Since |
Refactor statistics handling for Parquet filtering to improve row-group pruning efficiency. Signed-off-by: a12san <sainimal1ba@gmail.com>
Remove unnecessary blank line at the beginning of the file. Signed-off-by: a12san <sainimal1ba@gmail.com>
…tests Refactor tests to use _add_statistics_bounds instead of _derived_statistics_predicate for better handling of predicates. Update test cases to ensure compatibility with new bounds handling. Signed-off-by: a12san <sainimal1ba@gmail.com>
Done, pushed. The deriver is now _add_statistics_bounds. It walks the An OR with an unmodeled branch prunes now too. The modeled leaf stays in The filter also gets split once now. A filter with no modeled leaf comes Added two regression tests: one counting split passes for a plain |
|
The two bounds added here describe the leaf exactly ( Let's split this into 2 PRs: This PR
Follow-up PR
|
The bounds were only necessary conditions, so they could not prune under ~ and never marked a group fully matched: ~((x + 1) >= 901) kept every row group and filter((x + 1) >= 1) drove no limit push-down. Replace each modeled leaf by the exact range it spans: starts_with(p) becomes col >= p and col < next(p), and (col + c) >= t becomes col >= t - c, plus col <= MAX - c on signed columns whose addition computes at the column's width and wraps. Literals whose kernel path differs by operand order keep the leaf and only get the necessary lower bound appended. The rewrite is now semantically equal to the original filter, so NOT recurses into it and the fully-matched test runs on the rewritten filter, where the modeled leaves stay evaluable. Signed-off-by: sainimal1ba-hue <230497022+sainimal1ba-hue@users.noreply.github.com>
A file column can be narrower than the schema the filter runs at (an int8 file next to int64 files), where sums past the column's maximum do not wrap, so the exact two-sided range dropped matching rows. The rewritten filter keeps the original leaf and lets the evaluation decide. Also moves _add_statistics_bounds into formats/parquet/statistics_bounds.py with one helper per leaf shape, and consolidates its tests into test_statistics_bounds.py, one test per behavior.

Summary
Parquet statistics pruning currently misses filters such as (col("x") + 1) >= 76 and col("s").str.starts_with("s08"): PyArrow's row-group pruning only evaluates column-vs-literal comparisons against statistics, so anything else keeps every row group. This rewrites those leaves into the exact column ranges they span, so pruning, negation and the fully-matched test all see comparisons PyArrow can evaluate.
Design
For each leaf PyArrow cannot prune with:
Because each bound is equivalent to its leaf, nulls included, the leaf can be replaced outright. The rewritten filter is semantically equal to the original, so NOT recurses into it and the fully-matched test (~filter against statistics) runs on the rewritten filter: exact survivor counts drive limit push-down. Filters with no modeled leaf come back unchanged, so nothing else changes for them.
Before / after
Before: Wrapped arithmetic and prefix filters kept every row group. ~((x + 1) >= 901) pruned nothing, and on the issue's reproduction filter((x + 1) >= 1).limit(10) opened 112 files where the plain x >= 0 form opened 28.
After: FooterReader prunes with the exact ranges, groups fully inside the filter are marked fully matched, and ~ prunes too.
flowchart LR subgraph Before A["IO: Parquet"] --> B["FooterReader"] --> C["Unevaluable filter"] --> D["All row groups"] end subgraph After E["IO: Parquet"] --> F["FooterReader"] --> G["Exact statistics bounds"] --> H["Candidate row groups"] end D --> I["Original row filter"] H --> J["Original row filter"] classDef changed fill:#fff9c4,stroke:#f9a825,color:#000 class G,H changedSubsystem: Ray Data IO (Parquet footer reader).
Testing
Per the review, the remaining forms (<, <=, == on col + c, col - c, a literal on the left, col * c, float arithmetic, timestamp plus a duration) will follow in a separate PR, each with an exact bound from the start.
Fixes #67565.
AI assistance was used to develop this change. I tested and I have reviewed all the changes.