zhf999 opened a new pull request, #185:
URL: https://github.com/apache/paimon-cpp/pull/185
<!-- PR titles must follow Conventional Commits: <type>(<optional-scope>):
<description> -->
### Purpose
Reading a Parquet file with prefetch enabled and
`parquet.read.enable-page-index-filter` left at its
default (`true`) can fail with:
```
Invalid: seek to row failed. row number N should not be in the middle of
readable range
```
The prefetch coordinator and the page-filtered read path disagree about what
a reader position is:
- `PrefetchFileBatchReaderImpl` drives its sub readers with read ranges from
`ParquetFileBatchReader::GenReadRanges()`, which are **row group aligned**
and cover **all** row
groups, independent of any pushed down predicate.
- A partially matched row group returns its first *selected* row, which is
generally **inside** the
row group rather than at its start, and the sub reader crosses into the
next row group as soon as
the current one is exhausted, while the coordinator still points at the
previous read range.
`HandleReadResult()` then computes `slice_end == 0` ("fully out of range"),
drops the batch and
records that mid-row-group row as `readers_pos_`, expecting to seek back to
it later.
`FileReaderWrapper::SeekToRow()` only accepts row group boundaries, so the
following
`EnsureReaderPosition()` fails and the read is aborted. The drop-and-re-read
strategy comes from
ORC, whose `RowReader::seekToRow()` accepts an arbitrary row; it cannot work
for Parquet.
Two independent defects are fixed:
**1. `PrefetchFileBatchReaderImpl::HandleReadResult()` no longer drops a
batch that belongs to a
later read range.** It looks up the read range that actually contains the
batch and re-keys the batch
to it, so no backward seek is needed at all. Rows that belong to *another*
sub reader are still
dropped, which is safe because that reader produces them. The lookup is
restricted to
`read_ranges_in_group_[reader_idx]`, i.e. the ranges assigned to the
producing reader, so the
existing "one read range is served by exactly one queue" property is
preserved and output ordering is
unchanged. The recursion can happen at most once, because the first row id
lies inside the range
found and therefore cannot produce a zero slice end again.
**2. `FileReaderWrapper::NextPageFiltered()` now advances
`next_row_to_read_`.** It used to leave the
cursor at the row group start for the whole duration of a partially matched
row group, so
`GetNextRowToRead()` reported a position the reader had long passed. The
cursor now points at the
next row that survives filtering (mapped through the row group's
`RowRanges`), or at the row group
end once the filtered ranges are exhausted. This keeps the coordinator's
read range in step with the
sub reader, so the "fully out of range" path above becomes a pure fallback
instead of the common case.
Notes on behaviour:
- Output order is unchanged. Ordering is driven by the global `read_ranges_`
cursor, and read ranges
that yield no data are still retired by the existing `min_range` rule in
`NextBatchWithBitmap()`.
- Read ranges skipped by pushdown are now retired as soon as the first real
batch arrives, instead of
after a drop plus re-seek round trip, which removes one redundant row
group decode.
- ORC keeps its behaviour; it simply no longer re-reads a batch it has
already produced.
Known follow-up, intentionally left out:
`ParquetFileBatchReader::GenReadRanges()` still reports every
row group, including those already pruned by the predicate. Aligning it with
the surviving target row
groups would avoid empty read ranges, but it also changes range dispatch and
the adaptive prefetch
heuristic, so it deserves its own change.
### Tests
Two new cases, each verified to fail before the corresponding fix:
- `WriteAndReadInteTest.TestAppendWithParquetPageIndexFilterAndPrefetch`
(`test/inte`) - to verify that pre-metioned problem is solved.
- `FileReaderWrapperTest.PageFilteredAdvancesNextRowToRead`
(`paimon-parquet-format-test`) — to verify that `next_row_to_read_` is pushed
forward.
### API and Format
No.
### Documentation
No.
### Generative AI tooling
Generated-by: GLM 5.2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
