[core] Support file index and predicate push down in DataEvolutionSplitRead#8839
Open
wombatu-kun wants to merge 2 commits into
Open
[core] Support file index and predicate push down in DataEvolutionSplitRead#8839wombatu-kun wants to merge 2 commits into
wombatu-kun wants to merge 2 commits into
Conversation
…itRead DataEvolutionSplitRead.withFilter was a no-op, so data evolution tables never used their file indexes on read: AppendOnlyFileStoreScan evaluates the embedded index in filterByStats(ManifestEntry), but DataEvolutionFileStoreScan overrides that method and only checks row id ranges. Bloom, bitmap and bsi indexes were written for data evolution files and never read back. Format level push down did not reach the reader either, because the FormatReaderMapping.Builder was constructed without filters. Push down is now applied where it can not interfere with the column merging. A file read without merging gets the full treatment: the file index can skip it, a bitmap index result narrows the read to the matching positions, and the filters reach the format reader. A merged group only uses the file index to skip the whole group, since DataEvolutionFileReader zips its readers positionally and dropping rows in one of them would break the alignment. The existing enabledFilterPushDown flag of FormatReaderMapping.Builder already draws exactly that line, so no extra branching is needed for the format level part. Only plain data files may prove that a merged group can be skipped. mergeRangesAndSort guarantees they all span the row id range of the whole group, while a blob or vector-store file only covers a sub range and proves nothing about the other rows. Filters on row tracking fields are never pushed down. _ROW_ID and _SEQUENCE_NUMBER are assigned from the manifest entry rather than read from the file, and data evolution may reassign row ids, so a physical copy in the file can be stale. Dropping them also keeps filter devolution working, as it resolves predicate fields against the table schema, which has no system fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wombatu-kun
marked this pull request as draft
July 24, 2026 04:15
…on file Follow-up to the file index push down. A column missing from a data evolution file is not null, its values live in another file of the same row id range, so a predicate on it must not be pushed down. Formats such as Parquet read a column absent from the file schema as all null and drop every row, which silently lost rows once the scan pruned the group down to the file that does not carry the column. The single file path now pushes only the filters the file can answer, computed from its writeCols against the table fields. Its reader mappings move to a dedicated cache keyed by writeCols, so they can not collide with the merge path cache, which keys by the projected read field names and pushes no filters. New tests: a filter on a column absent from the read file returns all rows across parquet, orc and avro, and a bitmap selection composes correctly with a deletion vector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wombatu-kun
marked this pull request as ready for review
July 24, 2026 04:43
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.
Purpose
Resolves the TODO in
DataEvolutionSplitRead.withFilter, which was a no-op: Support File index push down (all conditions) and Predicate push down (only if no column merge).As a result, data evolution tables never used their file indexes on read:
AppendOnlyFileStoreScanevaluates the embedded index infilterByStats(ManifestEntry), butDataEvolutionFileStoreScanoverrides that method and only checks row id ranges. Bloom, bitmap and bsi indexes were written for data evolution files and never read back. Format level push down did not reach the reader either, since theFormatReaderMapping.Builderwas built withfilters = null.Push down is now applied wherever it can not interfere with the column merging:
BitmapIndexResultnarrows the read to the matching positions, and the filters reach the format reader, mirroringRawFileSplitRead.DataEvolutionFileReaderzips its inner readers positionally, so dropping rows in one of them would break the column alignment. Skipping the group is sound because a logical row takes each column from exactly one file.The existing
enabledFilterPushDownflag ofFormatReaderMapping.Builderalready draws that line (the merge path callsbuild(..., false)), so passingfiltersinto the builder needs no extra branching.Three safety points:
mergeRangesAndSortassertsareAllRangesSame(dataFiles), while a blob or vector-store file covers only a sub range and proves nothing about the other rows._ROW_IDand_SEQUENCE_NUMBERare never pushed down: they are assigned from the manifest entry, and data evolution may reassign row ids, so a physical copy in the file can be stale.writeCols, and the single file reader mappings use a dedicated cache keyed bywriteColsso they can not collide with the merge path cache.Semantics are unchanged:
withFilteris a hint, and this only drops rows proven not to match.Benchmark (throwaway, not in the diff): 200k rows in 40 row id groups, ids interleaved so min/max stats prune nothing, bloom filter, point filter. 39 of 40 groups skipped.
Left for a follow up: row level bitmap selection inside a merged group, which needs the bitmap translated into global row id ranges and fed through the existing
rowRangesmechanism.Tests
New
DataEvolutionFileIndexTest(11 tests). Readers are created directly from the splits withoutexecuteFilter(), so an empty result proves the reader itself pruned the rows; filter values sit inside the column min/max range so the scan does not drop the split first.Covered: a single file group skipped by a standalone
.indexfile, a merged group skipped by an embedded index, column alignment preserved in a merged group, exact bitmap index selection, a merged group skipped afterRENAME COLUMN(the filter has to be devolved by field id), a filter on a column absent from the read file returns all rows across parquet, orc and avro, a bitmap selection composed with a deletion vector,file-index.read.enabled = false, and a filter mixing_ROW_IDwith a data column.