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 <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 15 Jan 2025 15:29:53 +0000
Gerrit-HasComments: Yes

Reply via email to