Gabriel39 commented on code in PR #68780:
URL: https://github.com/apache/doris/pull/68780#discussion_r4225340653
##########
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:
Fixed in c15bcf891a. The shared `validate_offset_index()` now requires the
declared data-page ranges to be contiguous from `data_page_offset` through the
column chunk end. Both `load_native_page_indexes()` and
`load_native_offset_indexes()` already apply this check before publishing
indexes. A gap or uncovered suffix therefore disables the optional index pair
before page pruning or indexed navigation can omit data. Dictionary prefixes
remain allowed. Auxiliary-page gaps and trailing writer padding conservatively
disable the optional index and keep sequential traversal; the check adds no
file I/O.
Before the fix, the mixed V2/V1 regression returned the next physical row,
and the Arrow-written complete-file regression lost its only predicate match
when a matching ColumnIndex/OffsetIndex pair omitted the middle or final page.
These now return the exact expected values. Coverage includes
first/middle/final omissions, PLAIN/dictionary encoding, indexes
enabled/disabled, single-row batches, valid contiguous dictionary prefixes,
auxiliary-page gaps, and trailing padding. Existing pruning-profile tests still
verify that valid contiguous indexes retain page pruning.
All 746 related BE ASAN tests passed
(`*Parquet*:FileScannerV2Test.*:TableReaderTest.*`), as did clang-format 16 on
all PR-affected C++ files and `git diff --check`. The legacy reader under
`be/src/format` is unchanged.
--
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]
