Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/22407 )
Change subject: IMPALA-12588: Don't UPDATE rows that already have the desired value ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/22407/2/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java File fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java: http://gerrit.cloudera.org:8080/#/c/22407/2/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@54 PS2, Line 54: needsWherePredicateRewrite_ > To me, this name suggests that there are cases where the WHERE predicate ne If I'm not mistaken this flag is responsible for avoiding multiple rewrites; in this case, it might be better to implement the reset() method for this class and store and restore the original assignments in case of a reset/reanalyze. http://gerrit.cloudera.org:8080/#/c/22407/2/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@142 PS2, Line 142: (IMPALA-12588) nit: I think it's not necessary to include it here, the ticket ID is visible through git blame. http://gerrit.cloudera.org:8080/#/c/22407/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test: http://gerrit.cloudera.org:8080/#/c/22407/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-update-basic.test@256 PS2, Line 256: s nit: capital s http://gerrit.cloudera.org:8080/#/c/22407/2/testdata/workloads/functional-query/queries/QueryTest/kudu_update.test File testdata/workloads/functional-query/queries/QueryTest/kudu_update.test: http://gerrit.cloudera.org:8080/#/c/22407/2/testdata/workloads/functional-query/queries/QueryTest/kudu_update.test@27 PS2, Line 27: IMPALA-12588 nit: I think we mention the ticket ID when it's a regression fix http://gerrit.cloudera.org:8080/#/c/22407/2/testdata/workloads/functional-query/queries/QueryTest/kudu_update.test@252 PS2, Line 252: t nit: capital t http://gerrit.cloudera.org:8080/#/c/22407/2/testdata/workloads/functional-query/queries/QueryTest/kudu_update.test@253 PS2, Line 253: # predicate on non-key can be removed -- To view, visit http://gerrit.cloudera.org:8080/22407 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I926c80e8110de5a4615a3624a81a330f54317c8b Gerrit-Change-Number: 22407 Gerrit-PatchSet: 2 Gerrit-Owner: Noemi Pap-Takacs <[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, 12 Feb 2025 22:31:47 +0000 Gerrit-HasComments: Yes
