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/07/17 11:20:49 UTC

[Impala-ASF-CR] IMPALA-15191: Fix row id virtual column in USING clause

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


Change subject: IMPALA-15191: Fix _row_id virtual column in USING clause
......................................................................

IMPALA-15191: Fix _row_id virtual column in USING clause

_row_id and _last_updated_sequence_number are Iceberg V3 syntactic-sugar
virtual columns that are expanded during analysis to COALESCE()
expressions by the mandatory IcebergVirtualColumnRewriteRule, so they
never reach the planner or backend.

That rewrite runs only once, between the first analyze() and the
re-analysis that follows expr rewriting. A JOIN ... USING (col) clause,
however, is re-generated into an on-clause on every analysis pass
(TableRef.analyzeJoin()), and reset() discards the previously built
on-clause when a USING clause is present. As a result the rewritten
COALESCE() predicate produced in the first pass was thrown away during
re-analysis and replaced by a raw equi-predicate referencing the
ICEBERG_ROW_ID virtual column. That column then reached the scan node,
where it hit DCHECK(false) in HdfsScanPlanNode.

This patch expands the syntactic-sugar virtual columns into their
COALESCE() equivalents at the point where the USING clause builds the
equi-join predicate, instead of relying on the one-shot expr rewrite.

Testing:
* New e2e regression tests

Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Assisted-by: Claude Opus 4.8 (1M context)
---
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-row-lineage-join.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-row-lineage.test
M tests/query_test/test_iceberg.py
4 files changed, 129 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/24596/1
-- 
To view, visit http://gerrit.cloudera.org:8080/24596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Gerrit-Change-Number: 24596
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15191: Fix row id virtual column in USING clause

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.

Hello Peter Rozsa, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15191: Fix _row_id virtual column in USING clause
......................................................................

IMPALA-15191: Fix _row_id virtual column in USING clause

_row_id and _last_updated_sequence_number are Iceberg V3 syntactic-sugar
virtual columns that are expanded during analysis to COALESCE()
expressions by the mandatory IcebergVirtualColumnRewriteRule, so they
never reach the planner or backend.

That rewrite runs only once, between the first analyze() and the
re-analysis that follows expr rewriting. A JOIN ... USING (col) clause,
however, is re-generated into an on-clause on every analysis pass
(TableRef.analyzeJoin()), and reset() discards the previously built
on-clause when a USING clause is present. As a result the rewritten
COALESCE() predicate produced in the first pass was thrown away during
re-analysis and replaced by a raw equi-predicate referencing the
ICEBERG_ROW_ID virtual column. That column then reached the scan node,
where it hit DCHECK(false) in HdfsScanPlanNode.

This patch expands the syntactic-sugar virtual columns into their
COALESCE() equivalents at the point where the USING clause builds the
equi-join predicate, instead of relying on the one-shot expr rewrite.

Testing:
* New e2e regression tests

Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Assisted-by: Claude Opus 4.8 (1M context)
---
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-row-lineage-join.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-row-lineage.test
M tests/query_test/test_iceberg.py
4 files changed, 128 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/24596/2
-- 
To view, visit http://gerrit.cloudera.org:8080/24596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Gerrit-Change-Number: 24596
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-15191: Fix row id virtual column in USING clause

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24596 )

Change subject: IMPALA-15191: Fix _row_id virtual column in USING clause
......................................................................


Patch Set 2:

Looks good to me, but I'm going to retest since it's been awhile.


-- 
To view, visit http://gerrit.cloudera.org:8080/24596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Gerrit-Change-Number: 24596
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 21:22:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15191: Fix row id virtual column in USING clause

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24596 )

Change subject: IMPALA-15191: Fix _row_id virtual column in USING clause
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/24596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Gerrit-Change-Number: 24596
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 21:22:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15191: Fix row id virtual column in USING clause

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.

Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/24596 )

Change subject: IMPALA-15191: Fix _row_id virtual column in USING clause
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24596/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/24596/1/tests/query_test/test_iceberg.py@2387
PS1, Line 2387:     # IMPALA-15191: joining on the _row_id syntactic-sugar virtual column
nit: it's already commented in the test



-- 
To view, visit http://gerrit.cloudera.org:8080/24596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Gerrit-Change-Number: 24596
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Jul 2026 13:37:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15191: Fix row id virtual column in USING clause

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.

Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24596 )

Change subject: IMPALA-15191: Fix _row_id virtual column in USING clause
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/24596/2//COMMIT_MSG@11
PS2, Line 11: expressions by the mandatory IcebergVirtualColumnRewriteRule, so they
> The rewrite rule still covers other cases than a join using clause?
Yes, AFAICT all other cases are covered by the rewrite rule. They are tested in iceberg-v3-row-lineage.test and ranger_iceberg_v3_column_masking.test.



-- 
To view, visit http://gerrit.cloudera.org:8080/24596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1aef8e4030dc7ea25f38ca78219cde217bc3d84
Gerrit-Change-Number: 24596
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jul 2026 10:09:53 +0000
Gerrit-HasComments: Yes