Gabriel39 commented on code in PR #68780:
URL: https://github.com/apache/doris/pull/68780#discussion_r4217526742
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1067,11 +1067,11 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::load_dictionary_page(bool
template <bool IN_COLLECTION, bool OFFSET_INDEX>
Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::ensure_first_data_page_parsed() {
- if (_first_data_page_parsed) {
+ if (_first_data_page_parsed && _page_reader->has_active_offset_index()) {
Review Comment:
Fixed in 424cd94b60. `read_dictionary_filter()` now calls
`ensure_first_data_page_parsed()` before computing `right_row`, matching the
ordinary and fixed-width paths.
The Arrow-written multi-page regression now covers both PLAIN and dictionary
encodings, page indexes on/off, and batch sizes 5 and 64. It checks all
predicate values and projected payloads, plus counters proving that the
dictionary predicate and page-index paths were exercised. Before the fix, the
dictionary case failed the projection-mode consistency check; it now passes.
All 731 related BE ASAN tests and clang-format 16 checks passed.
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1067,11 +1067,11 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::load_dictionary_page(bool
template <bool IN_COLLECTION, bool OFFSET_INDEX>
Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::ensure_first_data_page_parsed() {
- if (_first_data_page_parsed) {
+ if (_first_data_page_parsed && _page_reader->has_active_offset_index()) {
return Status::OK();
}
- // OffsetIndex row bounds are untrusted until page zero has been
reconciled and its declared
- // cardinality checked, so no indexed skip may observe them before this
one-time parse.
+ // OffsetIndex bounds need an initial reconciliation. If the index is
discarded, every later
+ // page needs its header parsed before callers use its sequential row
bounds.
Review Comment:
Fixed in 424cd94b60. The column chunk reader records whether an indexed page
was skipped without parsing and validating its header. If the index is
subsequently discarded, it returns corruption before decoding further values,
because the current absolute row coordinate is no longer trustworthy. Valid
indexed skips remain lazy, and later fallback remains supported when preceding
page spans were verified.
Added a regression matrix for flat reads and nested seeks, including cache
hits/misses, the reported `[0,1,3]` index over pages containing `[1,1,2]` rows,
valid indexed skips, and safe late fallback. The original code accepted the
invalid transition; the fix rejects it, and the flat reader emits no value for
the uncertain row. Cache-hit counters are asserted.
All 731 related BE ASAN tests and clang-format 16 checks passed. The PR
description now documents the explicit rejection behavior.
--
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]
