You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2025/03/25 22:24:45 UTC

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22666


Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................

IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

A variety of Iceberg statements (including v2 deletes) rely
on getting information from the scan node child of the
delete node. Since tuple caching can insert a TupleCacheNode
above that scan, the logic is currently failing, because
it doesn't know how to bypass the TupleCacheNode and get
to the scan node below.

This modifies the logic in multiple places to detect a
TupleCacheNode and go past it to the get the scan node
below it.

Testing:
 - Added a basic Iceberg test with v2 deletes for the
  frontend test and custom cluster test

Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
---
M be/src/exec/iceberg-delete-builder.cc
M be/src/runtime/fragment-state.cc
M fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
M tests/custom_cluster/test_tuple_cache.py
5 files changed, 58 insertions(+), 5 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................

IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

A variety of Iceberg statements (including v2 deletes) rely
on getting information from the scan node child of the
delete node. Since tuple caching can insert a TupleCacheNode
above that scan, the logic is currently failing, because
it doesn't know how to bypass the TupleCacheNode and get
to the scan node below.

This modifies the logic in multiple places to detect a
TupleCacheNode and go past it to the get the scan node
below it.

Testing:
 - Added a basic Iceberg test with v2 deletes for the
  frontend test and custom cluster test

Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
---
M be/src/exec/iceberg-delete-builder.cc
M be/src/runtime/fragment-state.cc
M fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
M tests/custom_cluster/test_tuple_cache.py
5 files changed, 58 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22666/3/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/22666/3/be/src/exec/iceberg-delete-builder.cc@307
PS3, Line 307:     if (file_path->Ptr() == prev_file_path.Ptr() || *file_path == prev_file_path) {
> What led to this change?
Oh, looks like a rebase.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 22:35:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc@131
PS2, Line 131:         // Tuple caching can place a TupleCacheNode above the scan node. Look past
> I think this would make sense as a helper that could also be used in fragme
I turned this into a static helper function on PlanNode to look past a single tuple cache node.

Something that looks up a child with an option to bypass a tuple cache node would work here, but fragment-state.cc isn't looking for a child.


http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc@138
PS2, Line 138:         if (delete_scan_node->tnode_->node_type != TPlanNodeType::HDFS_SCAN_NODE) {
> Should this be a DCHECK? I think it would only happen if we've done somethi
Changed this to a DCHECK. I think the code in fragment-state.cc would fail at runtime before we would get to this DCHECK.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 07 Apr 2025 22:04:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc@131
PS2, Line 131:         // a TupleCacheNode to get to the scan node.
> I turned this into a static helper function on PlanNode to look past a sing
Ack


http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc@138
PS2, Line 138:         while (!q.empty()) q.pop();
> Changed this to a DCHECK. I think the code in fragment-state.cc would fail 
Done


http://gerrit.cloudera.org:8080/#/c/22666/3/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/22666/3/be/src/exec/iceberg-delete-builder.cc@307
PS3, Line 307:     if (file_path->Ptr() == prev_file_path.Ptr() || *file_path == prev_file_path) {
What led to this change?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 22:35:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18692/ : 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/22666
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 22:51:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/18854/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 01:09:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................

IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

A variety of Iceberg statements (including v2 deletes) rely
on getting information from the scan node child of the
delete node. Since tuple caching can insert a TupleCacheNode
above that scan, the logic is currently failing, because
it doesn't know how to bypass the TupleCacheNode and get
to the scan node below.

This modifies the logic in multiple places to detect a
TupleCacheNode and go past it to the get the scan node
below it.

Testing:
 - Added a basic Iceberg test with v2 deletes for the
  frontend test and custom cluster test

Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
---
M be/src/exec/exec-node.h
M be/src/exec/iceberg-delete-builder.cc
M be/src/runtime/fragment-state.cc
M fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
M tests/custom_cluster/test_tuple_cache.py
6 files changed, 60 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22666/3/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/22666/3/be/src/exec/iceberg-delete-builder.cc@307
PS3, Line 307:     if (file_path->Ptr() == prev_file_path.Ptr() || *file_path == prev_file_path) {
> Oh, looks like a rebase.
Yeah, my upload had my changes and a rebase mixed together. I forgot to keep them separate.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 22:38:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Apr 2025 18:48:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 1:

(1 comment)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22666/1/tests/custom_cluster/test_tuple_cache.py
File tests/custom_cluster/test_tuple_cache.py:

http://gerrit.cloudera.org:8080/#/c/22666/1/tests/custom_cluster/test_tuple_cache.py@772
PS1, Line 772: v
flake8: U100 Unused argument 'vector'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 22:26:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc@131
PS2, Line 131:         // Tuple caching can place a TupleCacheNode above the scan node. Look past
I think this would make sense as a helper that could also be used in fragment-state.cc, something like

  delete_scan_node = current->GetScanNode(0);


http://gerrit.cloudera.org:8080/#/c/22666/2/be/src/exec/iceberg-delete-builder.cc@138
PS2, Line 138:         if (delete_scan_node->tnode_->node_type != TPlanNodeType::HDFS_SCAN_NODE) {
Should this be a DCHECK? I think it would only happen if we've done something wrong in our code.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Mar 2025 22:26:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18724/ : 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/22666
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Mar 2025 19:40:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Apr 2025 00:20:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22666 )

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................

IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

A variety of Iceberg statements (including v2 deletes) rely
on getting information from the scan node child of the
delete node. Since tuple caching can insert a TupleCacheNode
above that scan, the logic is currently failing, because
it doesn't know how to bypass the TupleCacheNode and get
to the scan node below.

This modifies the logic in multiple places to detect a
TupleCacheNode and go past it to the get the scan node
below it.

Testing:
 - Added a basic Iceberg test with v2 deletes for the
  frontend test and custom cluster test

Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Reviewed-on: http://gerrit.cloudera.org:8080/22666
Reviewed-by: Kurt Deschler <kd...@cloudera.com>
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/exec-node.h
M be/src/exec/iceberg-delete-builder.cc
M be/src/runtime/fragment-state.cc
M fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
M tests/custom_cluster/test_tuple_cache.py
6 files changed, 60 insertions(+), 6 deletions(-)

Approvals:
  Kurt Deschler: Looks good to me, but someone else must approve
  Michael Smith: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-13882: Fix Iceberg v2 deletes with tuple caching

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

Change subject: IMPALA-13882: Fix Iceberg v2 deletes with tuple caching
......................................................................


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8
Gerrit-Change-Number: 22666
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 14:06:59 +0000
Gerrit-HasComments: No