github-actions[bot] commented on code in PR #68780:
URL: https://github.com/apache/doris/pull/68780#discussion_r4217819095
##########
be/src/format_v2/parquet/reader/native/column_reader.cpp:
##########
@@ -1470,10 +1470,12 @@ Status ScalarColumnReader<IN_COLLECTION,
OFFSET_INDEX>::read_dictionary_filter(
int64_t right_row = 0;
if constexpr (OFFSET_INDEX == false) {
RETURN_IF_ERROR(_chunk_reader->parse_page_header());
- right_row = _chunk_reader->page_end_row();
} else {
- right_row = _chunk_reader->page_end_row();
+ // An indexed reader can fall back at runtime; dictionary filtering
then needs the current
+ // page header to establish row bounds just like ordinary and
fixed-width reads.
+ RETURN_IF_ERROR(_chunk_reader->ensure_first_data_page_parsed());
Review Comment:
[P1] Reconcile a selected later page before using its indexed end row. After
page one has parsed, this guard returns early while an OffsetIndex is active.
With page indexes enabled, a non-pruning zonemap predicate, physical page
counts `[2,3,2]`, index starts `[0,2,4]`, selected rows `[2,5)`, and a stale
second-page byte size, this dictionary path stores `right_row=4` before
`parse_page_header()` drops the index and sets the true end to 5. It consumes
only two of that page's three values, advances, and can return the next page's
first value as logical row 4; row-count checks still pass. Ordinary and
fixed-width paths have the same ordering. This is distinct from the existing
first-page and skipped-page threads. Parse before forming ranges, or recompute
the bound and ranges when the index is discarded.
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1105,6 +1105,11 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::parse_page_header() {
}
}
const bool active_offset_index = _page_reader->has_active_offset_index();
Review Comment:
[P2] Keep a valid nested V1 page readable after an indexed skip. A Column
Chunk may have row-boundary V2 pages for rows 0 and 1, then a V1 page for row 2
with a correct OffsetIndex `[0,1,2]`. Selecting only row 2 skips page 1 and
sets `_skipped_unverified_indexed_page`; parsing page 2 then discards the index
solely because it is nested V1, so this new guard returns corruption for valid
data. This differs from the stale-index case in the existing thread. Preserve
safe indexed navigation for V1 or verify skipped spans before switching to
sequential mode, and cover this mixed-page seek.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
