github-actions[bot] commented on code in PR #68780:
URL: https://github.com/apache/doris/pull/68780#discussion_r4218682469
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1096,15 +1096,14 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::parse_page_header() {
}
int32_t page_num_values = _page_reader->is_header_v2() ?
header->data_page_header_v2.num_values
:
header->data_page_header.num_values;
- if constexpr (IN_COLLECTION && OFFSET_INDEX) {
- if (!_page_reader->is_header_v2() &&
_page_reader->has_active_offset_index()) {
- // V1 nested pages do not declare their logical row count. An
OffsetIndex span cannot
- // be trusted until repetition levels are decoded, so keep the
sequential cursor path.
- _page_reader->discard_offset_index();
- _offset_index = nullptr;
- }
- }
+ // A later V1 page can still use a valid OffsetIndex in a mixed-version
chunk. Its first
+ // repetition level is checked when loaded, and its row count when the
levels are exhausted.
const bool active_offset_index = _page_reader->has_active_offset_index();
+ if (_skipped_unverified_indexed_page && !active_offset_index) {
Review Comment:
[P2] Count earlier indexed values before a final V1 fallback. For a mixed
chunk with V2 row 0 and final V1 row 1, a stale compressed-size entry for that
last page discards the optional OffsetIndex. Earlier indexed pages never
incremented `_chunk_parsed_values`, so parsing the V1 page leaves the counter
at 1 although metadata declares 2 values. On level exhaustion,
`has_next_page()` falsely reports a successor and
`load_cross_page_nested_row()` parses past the Column Chunk, failing a scan
whose data pages are valid. Preserve the cumulative value count across a safe
index-to-sequential transition, and cover final nested V1 fallback.
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1096,15 +1096,14 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::parse_page_header() {
}
int32_t page_num_values = _page_reader->is_header_v2() ?
header->data_page_header_v2.num_values
:
header->data_page_header.num_values;
- if constexpr (IN_COLLECTION && OFFSET_INDEX) {
- if (!_page_reader->is_header_v2() &&
_page_reader->has_active_offset_index()) {
- // V1 nested pages do not declare their logical row count. An
OffsetIndex span cannot
- // be trusted until repetition levels are decoded, so keep the
sequential cursor path.
- _page_reader->discard_offset_index();
- _offset_index = nullptr;
- }
- }
+ // A later V1 page can still use a valid OffsetIndex in a mixed-version
chunk. Its first
Review Comment:
[P1] Validate a partly consumed nested V1 page before trusting the next
indexed row. After an indexed V2 first page, a sparse row-id request for rows 1
and 3 can leave a later V1 page partly consumed: it physically holds rows 1-3,
but its structurally accepted OffsetIndex advertises `[1,3)`.
`load_page_nested_rows` stops at the next row start after row 1, so its indexed
end-row check never runs. `next_page()` leaves the DATA_LOADED page without
marking it unverified, and `seek_to_nested_row(3)` assigns row 3 to the
following page's physical row 4. If that page is also read only partly, the
scan returns the wrong row without an error even while the index remains
active. Validate remaining V1 levels before leaving that page, or reject the
indexed jump; cover disjoint row selections.
--
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]
