github-actions[bot] commented on code in PR #66011:
URL: https://github.com/apache/doris/pull/66011#discussion_r3649192879
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -908,37 +907,54 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::read_levels(
}
template <bool IN_COLLECTION, bool OFFSET_INDEX>
-Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::_parse_first_page_header() {
+Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::_ensure_dictionary_page_loaded() {
+ if (_dict_checked) {
+ return Status::OK();
+ }
+
+ DORIS_CHECK(_state == INITIALIZED);
while (true) {
RETURN_IF_ERROR(_page_reader->parse_page_header());
const tparquet::PageHeader* header = nullptr;
RETURN_IF_ERROR(_page_reader->get_page_header(&header));
if (header->type == tparquet::PageType::DATA_PAGE ||
header->type == tparquet::PageType::DATA_PAGE_V2) {
- _state = INITIALIZED;
- return parse_page_header();
+ if constexpr (IN_COLLECTION && OFFSET_INDEX) {
+ if (header->type == tparquet::PageType::DATA_PAGE &&
+ _page_reader->has_active_offset_index()) {
+ // V1 nested pages expose row boundaries only in
repetition levels, so an
+ // indexed seek must not skip the first page before those
levels are decoded.
+ _page_reader->discard_offset_index();
+ _offset_index = nullptr;
+ }
+ }
+ _dict_checked = true;
Review Comment:
[P1] Establish page zero before consuming its indexed range
With lazy init, a flat reader can cache `page_end_row()` and decide to skip
page 0 before this helper parses its header. That parse may either leave the
index active without running `ColumnChunkReader::parse_page_header()`'s
`num_values`/`num_rows` checks, or invalidate a physically inconsistent index
after the caller has already cached its old row bound.
Both paths can shift rows. Indexed spans `[0,2), [2,4), [4,5), [5,6)` with
physical page counts `1,2,1,2` let a scan selecting `[2,4)` return physical
rows 1-2 as logical rows 2-3. Similarly, spans `[0,1), [1,3)` with physical
counts `2,1` plus a first-rectangle size mismatch switch to sequential end 2,
but the caller can still advance using cached end 1 and skip requested row 1.
Before this PR, `init()` established page 0 before any `page_end_row()` use.
Please preserve zero-I/O `init()`, but lazily reconcile and chunk-validate the
first data header before consuming its indexed range, with flat and nested-V2
skip/fallback tests.
--
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]
