fix(parquet): dense-fetch fallback when a projected column lacks page locations - #87
Open
fsdvh wants to merge 1 commit into
Open
fix(parquet): dense-fetch fallback when a projected column lacks page locations#87fsdvh wants to merge 1 commit into
fsdvh wants to merge 1 commit into
Conversation
…page locations The async row-selection reader defaults to the `Mask` selection strategy, which whenever an offset index is present fetches only the data pages a `RowSelection` touches (a "sparse" column chunk). `InMemoryRowGroup::fetch_ranges` took that sparse path for every projected column purely based on the offset index array being present, without checking that each column actually carries page locations. A column whose `OffsetIndexMetaData` has empty `page_locations` (for example metadata synthesized from a manifest that lacks a page index for that column) then produces no scan ranges: the column is fetched empty and the subsequent read fails with `Invalid offset in sparse column chunk data: <n>, no matching page found`. The existing `should_force_selectors` safety net does not cover this case (it returns false for empty page locations), and forcing `Selectors` would not help since the sparse fetch is chosen independently of the strategy. Guard the sparse path so it is only taken when every projected, not-yet-fetched column has non-empty page locations; otherwise fall back to fetching whole column chunks (dense), which is always correct. Columns lacking page locations that are not projected do not disable sparse fetches for the rest. Adds unit tests covering the sparse path, the dense fallback, and the unprojected-column case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Problem
The async row-selection reader defaults to the
Maskselection strategy. Whenever an offset index is present, it fetches only the data pages aRowSelectiontouches — a sparse column chunk (InMemoryRowGroup::fetch_ranges).That sparse path was taken for every projected column based purely on the offset-index array being present, without checking that each column actually carries page locations.
A column whose
OffsetIndexMetaDatahas emptypage_locations— e.g. metadata synthesized from a manifest that lacks a page index for that column — then produces no scan ranges viascan_ranges. The column is fetched empty and the subsequent read fails with:The existing
should_force_selectorssafety net does not cover this case —selection_skips_any_pagereturnsfalsefor a column with empty page locations. And forcingSelectorswould not help anyway, because the sparse fetch infetch_rangesis chosen independently of the resolved selection strategy (get_offset_indexindata.rsderives it straight from the metadata).Fix
Guard the sparse path in
fetch_rangesso it is only taken when every projected, not-yet-fetched column has non-empty page locations. Otherwise fall back to fetching whole column chunks (dense), which is always correct. A column that lacks page locations but is not projected does not disable sparse fetches for the remaining columns.This restores the invariant that a sparse (Mask-strategy) fetch relies on: if the offset index drives the fetch, every fetched column has real page locations.
Tests
Adds unit tests on
fetch_ranges:The dense-fallback test fails without the guard (the sparse path is taken and the read would later fail with "no matching page found").
Context
Surfaced downstream in Coralogix's DataPrime query engine, which synthesizes
ParquetMetaDatafrom its own manifest format and can leave a column's offset index empty when the manifest carries no page index for it. Under arrow v57's Mask default, any row-selection scan (e.g. full-text index selections) projecting such a column failed.