Skip to content

[core] Consider deletion vectors when selecting row sidecars - #10792

Open
lilei1128 wants to merge 3 commits into
apache:masterfrom
lilei1128:rowsidecar-with-dv
Open

lilei1128 wants to merge 3 commits into
apache:masterfrom
lilei1128:rowsidecar-with-dv

Conversation

@lilei1128

Copy link
Copy Markdown
Contributor

Purpose

Row sidecar selection estimated the selected row count from the requested
row ranges and, when available, a bitmap file-index result. A deletion
vector influenced that estimate only indirectly: the caller had already
merged it into the bitmap result, and only for 32-bit deletion vectors.
Without a file index, or with a 64-bit deletion vector, deleted rows were
therefore counted as live, and the heuristic could reject a row sidecar
that would actually have paid off.

Pass the group's DeletionVectorWithRange down to
shouldReadRowSidecar/selectedRowCount and compute the live row count
explicitly:

  • use rowCount - getCardinality() when the deletion vector covers exactly
    the file, avoiding a full scan;
  • otherwise scan the smaller of the selection and the deletion vector;
  • keep the existing behavior when the deletion vector is empty.

Applying the deletion vector on top of an already intersected file-index
result is idempotent, so the read-path preprocessing and the actual reads
are unchanged, while both 32-bit and 64-bit deletion vectors are now
handled. The change only affects the sidecar heuristic; rows are still
filtered by ApplyDeletionVectorReader at read time.

Tests

Add unit tests in DataEvolutionSplitReadTest covering:

  • no file index: live rows after deletion drive the decision;
  • anchor-range offset between the file and the deletion vector range,
    including positions outside the file;
  • the sparse deletion vector scan when the selection is larger than the
    deletion vector.

if (selection == null) {
if (deletionVectorRange != null
&& deletionVectorRange.equals(file.nonNullRowIdRange())) {
return file.rowCount() - deletionVector.getCardinality();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Base the sidecar decision on the selection actually passed to the reader

Subtracting the DV here changes the estimate without narrowing the format reader's selection. createFileReader still builds FormatReaderContext.selection only from rowRanges and the bitmap file-index result; ApplyDeletionVectorReader filters deleted rows after the underlying rows have been read.

With no bitmap selection, a 20,000-row file with 19,000 deleted rows now passes the default 4,096-row / 5% thresholds and selects .row, but the row reader still reads and decodes all 20,000 rows. I reproduced this with both 32-bit and 64-bit DVs; the baseline selects Parquet for the same inputs. This lets dense scans bypass the sparse-sidecar safeguards and switch to full row-store decoding, which is especially costly when projecting a few columns from wide rows.

Please either push the DV into the actual reader selection and reuse that selection for the heuristic, or retain the original estimate until that pushdown is available. An end-to-end test should verify the selection or physical rows/blocks read, rather than just the live-row count.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by pushing the DV into the reader selection and reused it for the heuristic: readTarget builds the selection once and carries it in FileReadTarget, so the sidecar decision and the .row reader share the same live-rows bitmap.
The pushdown is sidecar-only, so the Parquet path is unchanged.

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.

2 participants