You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2026/09/18 13:56:25 UTC

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24886


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 functional_parquet.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.

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 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
A testdata/workloads/functional-query/queries/QueryTest/parquet-late-materialization-collections.test
M tests/query_test/test_parquet_late_materialization.py
3 files changed, 56 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/24886/1
-- 
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: newchange
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/14128/ DRY_RUN=false


-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 14:36:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 2: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/24886/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24886/2//COMMIT_MSG@25
PS2, Line 25: returned positions 1, 4, 7, 0 instead of 1, 3, 5, 0; 7 is past the end of the
nit: wrap at 72? somehow claude doesn't seem to respect this rule


http://gerrit.cloudera.org:8080/#/c/24886/2/tests/query_test/test_parquet_late_materialization.py
File tests/query_test/test_parquet_late_materialization.py:

http://gerrit.cloudera.org:8080/#/c/24886/2/tests/query_test/test_parquet_late_materialization.py@43
PS2, Line 43: customer_nested_multiblock_multipage
nit: as we are using this at multiple tests, probably it would be better to add it during dataload



-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 09:24:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Xuebin Su (Code Review)" <ge...@cloudera.org>.
Xuebin Su has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 1: Code-Review+1

(1 comment)

> Uploaded patch set 1.

Thanks!

http://gerrit.cloudera.org:8080/#/c/24886/1/be/src/exec/parquet/parquet-column-readers.h
File be/src/exec/parquet/parquet-column-readers.h:

http://gerrit.cloudera.org:8080/#/c/24886/1/be/src/exec/parquet/parquet-column-readers.h@449
PS1, Line 449:   /// with the rep/def levels.
nit: How about adding some comments to elaborate?



-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 02:50:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 2:

(1 comment)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/24886/1/be/src/exec/parquet/parquet-column-readers.h
File be/src/exec/parquet/parquet-column-readers.h:

http://gerrit.cloudera.org:8080/#/c/24886/1/be/src/exec/parquet/parquet-column-readers.h@449
PS1, Line 449:   /// means that we already processed the Nth row completely, hence the initial value is
> nit: How about adding some comments to elaborate?
Updated the comment.



-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 08:48:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24886 )

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.

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 test_parquet_late_materialization.py, test_nested_types.py and
   test_parquet_stats.py.
 - customer_nested_multiblock_multipage is now loaded during dataload.

Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Assisted-by: Claude Fable 5.1 (Claude Code)
Reviewed-on: http://gerrit.cloudera.org:8080/24886
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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(-)

Approvals:
  Csaba Ringhofer: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
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: merged
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/24148/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 14:56:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 4:

(2 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/24886/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24886/2//COMMIT_MSG@25
PS2, Line 25:   where id % 2 = 0;
> nit: wrap at 72? somehow claude doesn't seem to respect this rule
Done


http://gerrit.cloudera.org:8080/#/c/24886/2/tests/query_test/test_parquet_late_materialization.py
File tests/query_test/test_parquet_late_materialization.py:

http://gerrit.cloudera.org:8080/#/c/24886/2/tests/query_test/test_parquet_late_materialization.py@43
PS2, Line 43: ratch batches get filtered out betwe
> nit: as we are using this at multiple tests, probably it would be better to
Done



-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 14:36:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 4: Verified+1


-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 19:11:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 2: Verified-1

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/24142/ : ABORTED


-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 11:47:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 4: Code-Review+2


-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Sep 2026 14:39:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 (#2).

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 functional_parquet.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.

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 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
A testdata/workloads/functional-query/queries/QueryTest/parquet-late-materialization-collections.test
M tests/query_test/test_parquet_late_materialization.py
3 files changed, 63 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/24886/2
-- 
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: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 (#4).

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.

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 test_parquet_late_materialization.py, test_nested_types.py and
   test_parquet_stats.py.
 - customer_nested_multiblock_multipage is now loaded during dataload.

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/4
-- 
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: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15374: Roll back current row when undoing a level read ahead

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24886 )

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


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/24086/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: comment
Gerrit-Change-Id: Icaf0aab770d87505513b0168f52f96480eda3202
Gerrit-Change-Number: 24886
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xuebin Su <xs...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Sep 2026 14:19:00 +0000
Gerrit-HasComments: No