github-actions[bot] commented on code in PR #68780:
URL: https://github.com/apache/doris/pull/68780#discussion_r4220674169
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1142,19 +1125,56 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::parse_page_header() {
_remaining_def_nums = page_num_values;
_remaining_num_values = page_num_values;
- // no offset will parse all header.
- if (!active_offset_index) {
- _chunk_parsed_values += _remaining_num_values;
- }
- _first_data_page_parsed = true;
+ // Count each parsed data header exactly once, including the indexed
prefix. A safe fallback
+ // uses this physical value count for EOF; nested logical rows are not
interchangeable with it.
+ _chunk_parsed_values += _remaining_num_values;
_state = HEADER_PARSED;
return Status::OK();
}
+template <bool IN_COLLECTION, bool OFFSET_INDEX>
+Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::_finish_indexed_nested_page() {
+ if constexpr (IN_COLLECTION && OFFSET_INDEX) {
+ if (_state != INITIALIZED && _page_reader->has_active_offset_index() &&
+ !_page_reader->is_header_v2()) {
+ // A V1 header has no logical row count. Loading only a prefix
does not validate its
+ // indexed span: drain the remaining level runs before trusting
the next indexed row.
+ // No value decoding or per-value scratch is needed for a page we
are leaving.
+ RETURN_IF_ERROR(load_page_data_idempotent());
+ SCOPED_RAW_TIMER(&_chunk_statistics.decode_level_time);
+ const size_t end_row = _page_reader->end_row();
+ while (_remaining_rep_nums > 0) {
+ level_t level = -1;
+ const size_t run = _rep_level_decoder.get_next_run(&level,
_remaining_rep_nums);
+ if (run == 0) {
+ return Status::Corruption(
+ "Parquet repetition level stream ended while
leaving an indexed page");
+ }
+ if (level == 0) {
+ if (_current_row > end_row || run > end_row -
_current_row) {
+ return Status::Corruption(
+ "Parquet nested page exceeds its indexed row
span");
+ }
+ _current_row += run;
+ }
+ _remaining_rep_nums -= run;
+ }
+ if (_current_row != end_row) {
+ return Status::Corruption(
+ "Parquet nested page does not match its indexed row
span");
+ }
+ }
+ }
+ return Status::OK();
+}
+
template <bool IN_COLLECTION, bool OFFSET_INDEX>
Status ColumnChunkReader<IN_COLLECTION, OFFSET_INDEX>::next_page() {
if constexpr (OFFSET_INDEX) {
- RETURN_IF_ERROR(ensure_first_data_page_parsed());
+ // A selected page's local span cannot prove its absolute row
coordinate. Validate
+ // every preceding page before advancing, even if its values are
entirely skipped.
+ RETURN_IF_ERROR(parse_page_header());
Review Comment:
[P1] Validate index completeness before page-level pruning. This checks each
listed page but never inspects a byte gap that may contain an omitted data
page; `validate_offset_index()` accepts the gap and a matching incomplete
ColumnIndex passes the loader. With physical V2 pages for rows 0 and 1 followed
by a V1 page for rows 2-3, indexes listing only the first and last page can
return physical row 2 as logical row 1 for a one-row selected range, before the
V1 span check runs. An omitted page containing the only predicate match can
even be pruned before this reader opens. Verify that index gaps contain no data
pages, or disable the page indexes, before planning and indexed navigation.
--
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]
