You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Peter Rozsa (Code Review)" <ge...@cloudera.org> on 2024/10/15 12:34:25 UTC

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Peter Rozsa has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21931


Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE
statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/main/java/org/apache/impala/analysis/MergeStmt.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
4 files changed, 247 insertions(+), 9 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:38:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE
statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
3 files changed, 245 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 3:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/21931/2//COMMIT_MSG@7
PS2, Line 7: E statements with INSERT merge clauses
           : 
> nit: no need to break line in title
Done


http://gerrit.cloudera.org:8080/#/c/21931/2//COMMIT_MSG@10
PS2, Line 10: including them only if there are UPDATE or DELETE merge clauses that are
            : listed in a MERGE statement. These fields can be omitted when there's no
            : delete file creation at the 
> Could you please explain in the commit message why this is needed? How did 
Done


http://gerrit.cloudera.org:8080/#/c/21931/2/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:

http://gerrit.cloudera.org:8080/#/c/21931/2/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@335
PS2, Line 335:     } else {
> nit: you could add a short comment that DELETE/UPDATE cases require positio
Done


http://gerrit.cloudera.org:8080/#/c/21931/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test:

http://gerrit.cloudera.org:8080/#/c/21931/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test@107
PS2, Line 107: es: 01(GETNEX
> +1
Added a new partitioned test and also added the extended information for proper validation



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 15:29:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:54:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21931/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test:

http://gerrit.cloudera.org:8080/#/c/21931/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test@271
PS3, Line 271: a 
> nit: a
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 16:23:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement. These fields can be omitted when there's no
delete file creation at the sink of the MERGE statement.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
3 files changed, 378 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21931/4
-- 
To view, visit http://gerrit.cloudera.org:8080/21931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 9: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11334/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 21:01:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jan 2025 17:04:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 14:18:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21931/5/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:

http://gerrit.cloudera.org:8080/#/c/21931/5/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@132
PS5, Line 132:           "MERGE statement is not supported for Iceberg tables "
> line too long (94 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:11:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jan 2025 11:33:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 08:51:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 8: Code-Review+2

Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:35:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement. These fields can be omitted when there's no
delete file creation at the sink of the MERGE statement.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
3 files changed, 386 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21931/6
-- 
To view, visit http://gerrit.cloudera.org:8080/21931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 15 Oct 2024 12:59:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21931/10/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:

http://gerrit.cloudera.org:8080/#/c/21931/10/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@288
PS10, Line 288: true,
> This was set to true by IMPALA-13656, wasn't it?
PS 10 is based on a commit before IMPALA-13656



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jan 2025 10:16:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement or the target table has existing delete
files. These fields can be omitted when there's no delete file creation
at the sink of the MERGE statement and the table has no existing delete
files.

Additionally, this change disables MERGE for Iceberg target tables
that contain equality delete files, see IMPALA-13674.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Reviewed-on: http://gerrit.cloudera.org:8080/21931
Reviewed-by: Noemi Pap-Takacs <np...@cloudera.com>
Reviewed-by: Daniel Becker <da...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
4 files changed, 376 insertions(+), 7 deletions(-)

Approvals:
  Noemi Pap-Takacs: Looks good to me, but someone else must approve
  Daniel Becker: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 12
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 16:49:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 2: Code-Review+1

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/21931/2//COMMIT_MSG@7
PS2, Line 7: E
           : s
nit: no need to break line in title


http://gerrit.cloudera.org:8080/#/c/21931/2/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:

http://gerrit.cloudera.org:8080/#/c/21931/2/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@335
PS2, Line 335:     } else {
nit: you could add a short comment that DELETE/UPDATE cases require position delete information.


http://gerrit.cloudera.org:8080/#/c/21931/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test:

http://gerrit.cloudera.org:8080/#/c/21931/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test@107
PS2, Line 107: unpartitioned
> Do you think it would make sense to add a test case with a partitioned targ
+1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 15:00:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 15:55:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11329/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 21:30:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:35:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement. These fields can be omitted when there's no
delete file creation at the sink of the MERGE statement and the table
has no existing delete files.

Additionally, this change disables MERGE for Iceberg target tables
that contain equality delete files, see IMPALA-13674.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
4 files changed, 392 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21931/7
-- 
To view, visit http://gerrit.cloudera.org:8080/21931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 10:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Jan 2025 15:07:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

Thanks, LGTM

http://gerrit.cloudera.org:8080/#/c/21931/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test:

http://gerrit.cloudera.org:8080/#/c/21931/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test@271
PS3, Line 271: an
nit: a



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 15:42:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 16:47:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 15:31:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 5:

(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/21931/5/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:

http://gerrit.cloudera.org:8080/#/c/21931/5/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@132
PS5, Line 132:           "MERGE statement is not supported for Iceberg tables containing equality deletes.");
line too long (94 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:09:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement or the target table has existing delete
files. These fields can be omitted when there's no delete file creation
at the sink of the MERGE statement and the table has no existing delete
files.

Additionally, this change disables MERGE for Iceberg target tables
that contain equality delete files, see IMPALA-13674.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
4 files changed, 392 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21931/8
-- 
To view, visit http://gerrit.cloudera.org:8080/21931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:33:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/21931/2//COMMIT_MSG@10
PS2, Line 10: This patch makes the Iceberg position field inclusion conditional by
            : including them only if there are UPDATE or DELETE merge clauses that are
            : listed in a MERGE statement.
Could you please explain in the commit message why this is needed? How did MERGE work before, was it incorrect or just unnecessary to include the position field? I saw that tests merge partitioned source to unpartitioned target tables. Does this have a relevance? If so, please explain here, if not, please cover the other cases.


http://gerrit.cloudera.org:8080/#/c/21931/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test:

http://gerrit.cloudera.org:8080/#/c/21931/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test@107
PS2, Line 107: unpartitioned
Do you think it would make sense to add a test case with a partitioned target table as well?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 14:34:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2025 16:26:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21931/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21931/6//COMMIT_MSG@11
PS6, Line 11: listed in a MERGE statement. These fields can be omitted when there's no
If I understand it correctly, we can only leave these fields out if there's no DELETE or UPDATE, and, no existing delete files either. This should be included in the commit message.


http://gerrit.cloudera.org:8080/#/c/21931/6//COMMIT_MSG@13
PS6, Line 13: 
We should also include that we throw an exception if there are EQ-delete files (which crashed so far).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:19:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:55:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 11: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jan 2025 10:32:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jan 2025 11:45:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement or the target table has existing delete
files. These fields can be omitted when there's no delete file creation
at the sink of the MERGE statement and the table has no existing delete
files.

Additionally, this change disables MERGE for Iceberg target tables
that contain equality delete files, see IMPALA-13674.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
4 files changed, 376 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21931/10
-- 
To view, visit http://gerrit.cloudera.org:8080/21931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement. These fields can be omitted when there's no
delete file creation at the sink of the MERGE statement.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
3 files changed, 378 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21931 )

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................

IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

This patch makes the Iceberg position field inclusion conditional by
including them only if there are UPDATE or DELETE merge clauses that are
listed in a MERGE statement. These fields can be omitted when there's no
delete file creation at the sink of the MERGE statement.

Tests:
 - iceberg-merge-insert-only planner test added

Change-Id: Ib62c78dab557625fa86988559b3732591755106f
---
M fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-merge-insert-only.test
3 files changed, 385 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21931/5
-- 
To view, visit http://gerrit.cloudera.org:8080/21931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21931/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21931/6//COMMIT_MSG@11
PS6, Line 11: listed in a MERGE statement. These fields can be omitted when there's no
> If I understand it correctly, we can only leave these fields out if there's
Done


http://gerrit.cloudera.org:8080/#/c/21931/6//COMMIT_MSG@13
PS6, Line 13: has no existing delete files.
> We should also include that we throw an exception if there are EQ-delete fi
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:28:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2025 15:35:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses

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

Change subject: IMPALA-13205: Do not include Iceberg position fields for MERGE statements with INSERT merge clauses
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21931/10/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:

http://gerrit.cloudera.org:8080/#/c/21931/10/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@288
PS10, Line 288: false
This was set to true by IMPALA-13656, wasn't it?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62c78dab557625fa86988559b3732591755106f
Gerrit-Change-Number: 21931
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jan 2025 10:01:42 +0000
Gerrit-HasComments: Yes