Skip to content

Rewrite NOT in row filters before the Arrow scan - #4350

Open
rahulsmahadev wants to merge 2 commits into
apache:mainfrom
rahulsmahadev:rahul/scan-filter-rewrite-not
Open

rahulsmahadev wants to merge 2 commits into
apache:mainfrom
rahulsmahadev:rahul/scan-filter-rewrite-not

Conversation

@rahulsmahadev

Copy link
Copy Markdown
Contributor

Rationale for this change

The manifest and metrics evaluators apply rewrite_not, so not value < 6.0 decides which files are read as value >= 6.0. The rows of those files were then filtered with the original expression, which Arrow evaluates as ~(value < 6.0). The two disagree on NaN: value >= 6.0 is false for NaN, ~(value < 6.0) is true. A NaN row was therefore returned only when another value in its file kept the file from being skipped:

# file 1: value [NaN, 7.0] (ids 1, 2); file 2: value [NaN, 5.0] (ids 3, 4)
tbl.scan(row_filter="not value < 6.0").to_arrow()  # ids [1, 2]: the NaN of file 2 is skipped with its file
tbl.scan(row_filter="value >= 6.0").to_arrow()     # ids [2]

This change applies rewrite_not to the bound file filter in _task_to_record_batches too, so rows are filtered with the same predicates that decided which files to read, and NOT no longer reaches Arrow. Only negated range comparisons on NaN change: the other negations (=, IN, IS NULL, IS NaN, STARTS WITH) give the same Arrow result before and after the rewrite.

Follow-up from the review of #4346, as suggested by @Fokko.

Are these changes tested?

Yes. test_scan_negated_comparison_matches_file_pruning_for_nan in tests/catalog/test_catalog_behaviors.py scans a file that is read and a file that is skipped, both holding a NaN row, with not value < 6.0, not (value < 6.0 and id = 1) and their rewritten forms, on each catalog of the catalog fixture. It fails without this change.

Are there any user-facing changes?

Yes: a scan whose filter negates a range comparison on a float or double column no longer returns NaN rows, which matches the file-level filtering. No API changes.

This pull request and its description were written by Isaac.

rahulsmahadev and others added 2 commits November 11, 2026 20:48
The manifest and metrics evaluators apply rewrite_not, so not x < 6.0
decides which files are read as x >= 6.0, but the rows were filtered with
~(x < 6.0), which is true for NaN. Apply rewrite_not to the bound file
filter as well, so a NaN row no longer depends on whether its file was read.

Co-authored-by: Isaac <[email protected]>
Co-authored-by: Isaac <[email protected]>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant