[Data] Support predicate pushdown at the Delta Lake level - #65142
[Data] Support predicate pushdown at the Delta Lake level#65142tobby168 wants to merge 6 commits into
Conversation
Groundwork for deferring Delta Lake file resolution to execution time (ray-project#61547). `prune_add_actions` answers, from a Delta snapshot's add actions alone, which data files a query could need. Partition values are matched exactly via Ray's own expression evaluator; data columns are bounded conservatively against the log's min/max statistics, so a file is dropped only when its interval proves no row can match. Every unprovable case keeps the file -- over-retention costs a read, but dropping a matching file loses data silently. `DeltaFileIndexer` lists a table from that log rather than walking the filesystem, applying the pruning before files are sized or chunked. Paths are reconstructed by decoding the log's URL-encoded relative path once before joining; the raw value names a path that does not exist when a partition value contains `=`, `/` or a space. `build_manifests` is extracted from `NonSamplingFileIndexer` unchanged so both indexers share manifest batching. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tobby168 <tobby168@gmail.com>
`read_delta` now builds a `DeltaDatasourceV2` when `use_datasource_v2` is set (the default), so listing is deferred to execution and driven by the Delta log rather than `to_pyarrow_dataset()`, which resolved every file on the driver before any predicate was known (ray-project#61547). `PushdownDeltaFilePruning` runs after `PredicatePushdown` and copies the predicates it settled onto the scanner into the upstream `ListFiles` indexer. Files that cannot match are then never listed, sized, or scheduled. Pruning is strictly an optimization: the scanner still enforces the predicates, so a plan the rule does not recognize reads more files but returns the same rows. Schema and partition columns come from the log, so no Parquet footers are sampled. The scanner pins a projection to keep the Delta column order -- the reader appends path-derived partition columns, which would otherwise move them to the end of every partitioned table. Also handled: - Deletion Vectors are rejected up front on both paths. Reading those files directly returns rows the table marks deleted; the previous error text suggested upgrading to deltalake>=0.10.0, which is not the issue under 1.5.0. - `Partitioning.null_fallback` parses a sentinel directory name back to `None`. Delta writes null partition values as `__HIVE_DEFAULT_PARTITION__`, which cannot be cast to the column's declared type. Opt-in, so `read_parquet` is unchanged. - An empty Delta table is answered from the log instead of tripping the V2 "no files found" guard, which exists to catch empty directories. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tobby168 <tobby168@gmail.com>
Under `delta.columnMapping.mode`, a table's Parquet files carry generated column names and the logical names live only in the transaction log. The V2 path opens those files directly with the logical schema, so it would ask for names the files don't have and hand back empty columns. Route such tables to the V1 path instead of guarding against them. Today `to_pyarrow_dataset` raises for them, so behavior is unchanged; if ray-project#64929 lands, they pick up its fix without this path needing to know about it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tobby168 <tobby168@gmail.com>
Review of ray-project#65142 found the V2 read path diverging from V1 in ways that were silent, plus a documented invariant the code did not actually hold. `count()` crashed: `PushdownCountFiles` asserted the indexer was a `NonSamplingFileIndexer`. It only needs one that can be pinned to whole-file listing, so it now checks for that and skips the optimization otherwise instead of failing the query. The PR's own test hid this by reading before counting, which populated the cached row count. `**arrow_parquet_args` was accepted and dropped, changing which rows came back. `catalog=` broke: its bucket-rooted filesystem was wrapped in a table-rooted `SubTreeFileSystem` for `to_pyarrow_dataset`, but the V2 path resolves absolute paths from the log. Both now decide V2 eligibility in one place, `_delta_table_supports_datasource_v2`, which fails closed -- anything V2 would answer *differently* rather than merely faster stays on V1. The claim that log-level pruning is only an optimization did not hold for typed partition columns: partition values parsed from a path are strings, so `col("year") == lit(2024)` raised inside PyArrow and was conservatively read as "keep the file", leaving pruning as the only thing enforcing the predicate. The partitioning now carries `field_types` from the Delta schema so the scanner can evaluate such predicates itself, and tables whose partition types that cannot express (date, decimal) stay on V1. `PathPartitionParser` also no longer coerces a `null_fallback`-derived `None`, which the two features together would otherwise hit. Also: the snapshot version is pinned on the driver, so deferred listing can't pick up a concurrent commit or disagree across task retries; `path` survives a column projection under `include_paths`; an empty table honors `columns` and `include_paths`; the rule declares `PredicatePushdown` as a dependency rather than relying on list position; the datasource caches its snapshot instead of reopening the log five times per read; and listing uses `ParquetFileChunker`, matching `read_parquet`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tobby168 <tobby168@gmail.com>
A second review found the V2 Delta path returning extra rows in its
default configuration, and failing outright for URI-addressed tables.
`PathPartitionParser.evaluate_predicate_on_partition` swallows any
evaluation error and keeps the file. That is sound only while something
else applies the predicate -- but `ReadFiles.apply_predicate` removes the
`Filter` as soon as a scanner accepts one, leaving path parsing as the
sole enforcement point. A path is a lossy view of what the Delta log
records: a null partition is the literal `__HIVE_DEFAULT_PARTITION__`
directory, and `pc.is_in` over the null-typed row the parser builds
raises. `col("p").is_in(["a"]) | (col("p") == lit("b"))` over a nullable
partition column therefore returned the NULL partition's rows as well.
Scanners can now decline to enforce a partition predicate
(`enforces_partition_predicate`), in which case the optimizer keeps it in
a `Filter` above the read, where it binds against the partition columns
the reader has already materialized -- exactly typed, with real nulls.
`DeltaScanner` declines. `PushdownDeltaFilePruning` still forwards the
same predicate to the log for file skipping, so pruning is unchanged and
only the correctness burden moves; a partition-pruned read still lists 1
of 2 files.
This also removes the reason to keep exotic partition types off V2: they
now simply prune less rather than risk a wrong answer.
`DeltaDatasourceV2` also never resolved its paths, so manifests carried
scheme-prefixed URIs and PyArrow rejected them at read time -- breaking
every `s3://`/`az://` table, including `read_delta`'s own docstring
examples, with V2 on by default. The log is now opened with the URI as
given while the reader gets filesystem-native paths. This was also the
remaining half of the `catalog=` breakage.
Tables with a column named `path` stay on V1; that name is how the reader
labels the synthesized file path, so the real column was being shadowed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: tobby168 <tobby168@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a DataSourceV2 implementation for Delta Lake tables, enabling log-driven file listing and pruning (both partition pruning and statistics-based skipping) via DeltaFileIndexer and prune_add_actions. It also adds logical rules (PushdownDeltaFilePruning) to push down predicates settled by PredicatePushdown to the DeltaFileIndexer, and updates read_delta to route through the V2 path when use_datasource_v2 is enabled. The review feedback highlights several instances of redundant wrapping of PyArrow objects (such as pa.schema() and pa.table()) where the underlying methods already return the correct types.
| columns keep their declared position and type rather than being | ||
| appended as the strings the directory names encode. | ||
| """ | ||
| schema = pa.schema(self._open_table().schema().to_arrow()) |
There was a problem hiding this comment.
The call to pa.schema() is redundant here because self._open_table().schema().to_arrow() already returns a pyarrow.Schema object. Removing the redundant wrapping simplifies the code and avoids unnecessary object instantiation.
| schema = pa.schema(self._open_table().schema().to_arrow()) | |
| schema = self._open_table().schema().to_arrow() |
| version=self._version, | ||
| storage_options=self._storage_options, | ||
| ) | ||
| add_actions = pa.table(table.get_add_actions(flatten=True)) |
There was a problem hiding this comment.
The call to pa.table() is redundant here because table.get_add_actions(flatten=True) already returns a pyarrow.Table object. Removing the redundant wrapping simplifies the code and avoids unnecessary object instantiation.
| add_actions = pa.table(table.get_add_actions(flatten=True)) | |
| add_actions = table.get_add_actions(flatten=True) |
| if mode is not None and mode.lower() != "none": | ||
| return False | ||
|
|
||
| schema = pa.schema(dt.schema().to_arrow()) |
There was a problem hiding this comment.
The call to pa.schema() is redundant here because dt.schema().to_arrow() already returns a pyarrow.Schema object. Removing the redundant wrapping simplifies the code and avoids unnecessary object instantiation.
| schema = pa.schema(dt.schema().to_arrow()) | |
| schema = dt.schema().to_arrow() |
| if not partition_columns: | ||
| return {} | ||
|
|
||
| schema = pa.schema(dt.schema().to_arrow()) |
There was a problem hiding this comment.
The call to pa.schema() is redundant here because dt.schema().to_arrow() already returns a pyarrow.Schema object. Removing the redundant wrapping simplifies the code and avoids unnecessary object instantiation.
| schema = pa.schema(dt.schema().to_arrow()) | |
| schema = dt.schema().to_arrow() |
| INCLUDE_PATHS_COLUMN_NAME, | ||
| ) | ||
|
|
||
| schema = pyarrow.schema(dt.schema().to_arrow()) |
There was a problem hiding this comment.
The call to pyarrow.schema() is redundant here because dt.schema().to_arrow() already returns a pyarrow.Schema object. Removing the redundant wrapping simplifies the code and avoids unnecessary object instantiation.
| schema = pyarrow.schema(dt.schema().to_arrow()) | |
| schema = dt.schema().to_arrow() |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit 07c8fb0. Configure here.
| split.partition_predicate | ||
| if residual_predicate is None | ||
| else residual_predicate & split.partition_predicate | ||
| ) |
There was a problem hiding this comment.
Residual filter needs partition columns
Medium Severity
When enforces_partition_predicate is false, the partition predicate is kept as a residual Filter above ReadFiles. That filter needs the partition columns in each block, but a columns projection can omit them so the reader never synthesizes them. The residual filter then fails at execution for patterns like reading a subset of columns and filtering on a partition key.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 07c8fb0. Configure here.
`_empty_delta_dataset` appended the synthesized `path` column and then projected to `columns`, which dropped it again. A real read adds `path` after the projection instead, so `read_delta(columns=["a"], include_paths=True)` returned `["a", "path"]` for a populated table and `["a"]` for an empty one -- a union of the two then failed on mismatched schemas. Both paths now derive their output columns from `_delta_output_columns`, so they can't drift apart again, and the test covers each combination of `columns` and `include_paths` rather than one option at a time. Reported by Cursor Bugbot on ray-project#65142. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tobby168 <tobby168@gmail.com>
|
Thanks both — responses below, plus one behavior change worth flagging that came out of digging into the second Bugbot finding. @gemini-code-assist: the five
|
|
Thanks for the detailed breakdown, @tobby168. Your analysis of the Regarding the behavior change for partition columns in |
|
Drive-by note, not a review — this PR touches the same loop I was about to send a separate fix for, and they're different defects, so flagging here rather than opening a competing PR. Your f = PathPartitionFilter.of(lambda d: True if d else False, style="hive",
base_dir="/tmp/part_probe", field_types={"year": int})
f(["/tmp/part_probe/year=2024/f.csv", "/tmp/part_probe/loose.csv"])
# KeyError: 'year'Either works for me: I can wait for this to land and send the missing-key fix on top, or you could fold it in here since it's the same two lines. Whichever you prefer. |


Summary
read_deltaresolves the entire Delta table on the driver before any predicate is known:Every file is enumerated at call time. For tables that rely on Delta's Z-ordering and per-file statistics rather than Hive partitioning, a selective query still touches every file — at minimum for tail metadata.
This routes
read_deltathrough the DataSourceV2 pipeline (already the default) with a Delta-native file indexer that prunes files from the transaction log — by partition values and by per-file min/max statistics — before any file is listed, sized, or scheduled.On an unpartitioned table of 4 files holding disjoint
valranges (the shape the issue describes):val > 250val < 5val > 1000val >= 0val != 3Closes #61547.
Design
Log-level pruning is an optimization, never a correctness requirement.
Predicates reach the scanner through the existing
ReadFiles.apply_predicatepath and are enforced there while reading — data predicates through the PyArrow scanner filter, partition predicates by parsing the file path.PushdownDeltaFilePruningruns afterPredicatePushdownand additionally copies them onto the upstreamListFilesindexer, so files that cannot match are never listed. If that rule doesn't fire, the query reads more files but returns the same rows.Making that true rather than merely intended took two changes, both of which earlier revisions of this PR got wrong:
PathPartitionParser.evaluate_predicate_on_partitionswallows any evaluation error and keeps the file — sound only while something else applies the predicate, butapply_predicateremoves theFilteras soon as a scanner accepts one. A path is a lossy view of the log: a null partition is the literal__HIVE_DEFAULT_PARTITION__directory, andpc.is_inover the null-typed row the parser builds raises. Scanners can now decline enforcement viaenforces_partition_predicate;DeltaScannerdoes, so the predicate stays in aFilterthat binds against the partition columns the reader has already materialized — exactly typed, with real nulls. The rule still forwards it to the log for skipping, so a partition-pruned read still lists 1 of 2 files.PredicatePushdownindependencies().Rulesetonly honors declared edges; list position is a tiebreaker, so an ordering assumption left in a comment can be silently inverted by an unrelated rule.Statistics-based skipping is conservative: a file is dropped only when
[min, max]proves no row can match.!=,NOT, UDFs, missing stats, a null bound, an unknown column — every "don't know" keeps the file. Over-retention costs a read; dropping a matching file loses data silently.test_pruning_never_drops_a_matching_fileasserts that property over generated inputs (seeded per parameter, so each case is reproducible).DeltaScannersubclassesParquetScannerand adds exactly one thing — declining partition-predicate enforcement. Filter pushdown, column pruning and limit pushdown are inherited unchanged, and the reader isParquetFileReaderas-is, because a Delta table's data files are ordinary Parquet laid out Hive-style. Schema and partition columns come from the log, so no Parquet footers are sampled for inference.The snapshot version is resolved on the driver and pinned into the datasource. Listing is deferred to workers, so leaving it unresolved would let a concurrent commit be read against a schema inferred before it existed, and could make a retried task disagree with its first attempt.
When V2 is used
_delta_table_supports_datasource_v2is the single gate, and it fails closed — anything V2 would answer differently rather than merely faster stays on V1, so enabling V2 never changes a result:to_pyarrow_datasetcurrently raises for these, so behavior is unchanged — and if [data] Fix delta column mapping read #64929 lands they pick up its fix without this path needing to know about it.path. That is how the reader labels the synthesized file path, so a real column of that name would be shadowed rather than returned.**arrow_parquet_argspassed. V1 forwards these toiter_batches; V2 has nowhere to put them, so honoring them means staying on V1 rather than silently dropping them.Deletion Vectors are rejected up front on both paths. Those data files still physically contain the deleted rows, and the V2 path opens them directly. The previous error text suggested upgrading to
deltalake>=0.10.0, which is not the issue under 1.5.0.Incidental correctness fixes
Cases where the V2 path would otherwise have been silently wrong:
e=flives ingrp=e%3Dfbut is recorded asgrp=e%253Df. Joining the raw value yields a path that doesn't exist. Decoded once before joining, which reproducesDeltaTable.file_uris()without a second listing.pathadded back wheninclude_pathsis set so a column projection can't drop it.__HIVE_DEFAULT_PARTITION__, which cannot be cast to the column's declared type. Adds opt-inPartitioning.null_fallback, andPathPartitionParserno longer coerces the resultingNone.read_parquetbehavior is unchanged.deltalakeresolves its storage backend from the URI while PyArrow rejects a scheme-prefixed path, so the log is opened with the path as given and the reader gets filesystem-native paths. Without this everys3:///az://table —read_delta's own documented examples — failed at execution. This was also the remaining half of thecatalog=breakage.count().PushdownCountFilesasserted its indexer was aNonSamplingFileIndexer; it only needs one that can be pinned to whole-file listing. It now checks for that and skips the optimization otherwise, rather than failing the query.columnsandinclude_paths, so unioning it with a populated table doesn't fail on mismatched schemas — instead of tripping the V2 "no files found" guard, which exists to catch empty directories.Not a duplicate
No open PR addresses #61547 (
gh pr list --state open --search "61547 in:body"returns nothing). #62959 was an earlier attempt by the same author on a since-superseded architecture; it was closed by the stale bot without human review, and this is a fresh implementation against the current DataSourceV2 code.richardliawinvited work on the issue in this comment.The nearest open PR in this area, #64929 (Delta column mapping), touches
read_deltain the same function but solves a different problem; the column-mapping fallback above is written so the two compose rather than conflict.Testing
Run per file, matching how Bazel runs these targets. 270 passed, 0 failed.
pre-commit run --files $(git diff --name-only master...HEAD)— all hooks pass.Tests assert on files skipped, not only on rows: reading every file also produces the right answer, so a pruning optimization that silently stopped pruning would otherwise pass every correctness test. Likewise
test_count_matches_v1counts on a fresh dataset, because reading first populates the cached row count and hides the plan shapecount()actually takes.test_parquet.py,test_partitioning.pyandpython/ray/data/tests/unit/show pre-existing failures in the local environment used here (missingtorch, S3/moto fixtures, and Ray workers unable to import test modules outside Bazel). Each was verified identical on the base commit —test_parquet.py7 failed / 34 errors on both,test_partitioning.py32 failed / 6 errors on both,tests/unit/9 failed on both.AI assistance
This change was developed with AI assistance (Claude Code), including two adversarial review passes. The first found the
count(),**arrow_parquet_argsandcatalog=breakage; the second found that partition predicates still had a single, fail-open enforcement point — returning extra rows foris_inover a nullable partition column in the default configuration — and that URI-addressed tables did not work at all. Both are fixed and covered by tests above.