Hello Xuebin Su, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24886

to look at the new patch set (#3).

Change subject: IMPALA-15374: Roll back current_row_ when undoing a level read 
ahead
......................................................................

IMPALA-15374: Roll back current_row_ when undoing a level read ahead

Collection column readers drive their children through the
non-batched interface, so a child always has one level read ahead.
When the scanner skips rows for such a child,
BaseScalarColumnReader::SkipRows() undoes that read ahead but did not
roll back 'current_row_', which NextLevels() had already incremented
for the level being un-read. SkipTopLevelRows() then counted from a
starting point one too high, so the counter drifted by one per
skipped range within a row group.

Which symptom this produces depends on the page index. Without it the
values stay aligned but LastProcessedRow() is too high, so a
collection reader that fills the file position slot reports positions
that are too large:

  set parquet_late_materialization_threshold=1;
  select file__position, int_array, id from complextypestbl
  where id % 2 = 0;

returned positions 1, 4, 7, 0 instead of 1, 3, 5, 0; 7 is past the
end of the seven row file. With the page index the rows to skip are
computed as 'skip_row_id - LastProcessedRow()', so the reader skips
one row too few and falls behind the columns it is read with,
silently pairing rows with the wrong collection value:

  set batch_size=4;
  select c_custkey, count(o.o_orderkey), min(o.o_orderkey)
  from customer_nested_multiblock_multipage c left join c.c_orders o
  where c_custkey > 280 and c_custkey % 9 = 2 group by c_custkey;

returned customer 289's orders for customer 290, and none for
customer 299.

Roll the counter back under the same condition NextLevels() uses to
advance it, before 'rep_level_' is invalidated. Levels read ahead at
the end of a row group never advanced it and have 'rep_level_' ==
ROW_GROUP_END, so they are left alone.

Broken since IMPALA-3841, which added the read ahead undo along with
late materialization for collections. Only readers driven through the
non-batched interface are affected: top level scalar readers never
read a level ahead, and struct and VARIANT readers are still
excluded.

customer_nested_multiblock_multipage is now loaded during dataload,
the same way customer_multiblock is, instead of being created on
demand by each of the three tests that need it. It has the same
schema as customer_multiblock, but its data file has multiple row
groups and multiple pages per column chunk, so it is the one nested
table that can cross both boundaries.

Testing:
 - Added QueryTest/parquet-late-materialization-collections.test with
   a query per symptom, run with late materialization and the page
   index on and off. Both fail without the fix.
 - Ran testdata/bin/generate-schema-statements.py and checked the
   generated CREATE, LOAD and REFRESH statements.
 - Ran test_parquet_late_materialization.py, test_nested_types.py and
   test_parquet_stats.py.

Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Assisted-by: Claude Fable 5.1 (Claude Code)
---
M be/src/exec/parquet/parquet-column-readers.h
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A 
testdata/workloads/functional-query/queries/QueryTest/parquet-late-materialization-collections.test
M 
testdata/workloads/functional-query/queries/QueryTest/parquet-late-materialization-unique-db.test
M 
testdata/workloads/functional-query/queries/QueryTest/virtual-column-file-position-parquet.test
M tests/query_test/test_parquet_late_materialization.py
M tests/query_test/test_scanners.py
8 files changed, 94 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/24886/3
--
To view, visit http://gerrit.cloudera.org:8080/24886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Xuebin Su <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>

Reply via email to