Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/20477 )
Change subject: IMPALA-12313: (part 1) Refactor modify statements ...................................................................... Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/IcebergDeleteImpl.java File fe/src/main/java/org/apache/impala/analysis/IcebergDeleteImpl.java: http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/IcebergDeleteImpl.java@20 PS1, Line 20: import java.util.List; Unused import http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java File fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java: http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java@21 PS1, Line 21: import org.apache.impala.catalog.FeKuduTable; Unused import http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/KuduDeleteImpl.java File fe/src/main/java/org/apache/impala/analysis/KuduDeleteImpl.java: http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/KuduDeleteImpl.java@20 PS1, Line 20: import java.util.List; Unused import http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/KuduUpdateImpl.java File fe/src/main/java/org/apache/impala/analysis/KuduUpdateImpl.java: http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/KuduUpdateImpl.java@20 PS1, Line 20: import java.util.List; Unused import http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java: http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@26 PS1, Line 26: import java.util.Map; Unused imports http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@122 PS1, Line 122: * clause. It also identifies the target table. Raises errors for unsupported tables typo: table http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@212 PS1, Line 212: public QueryStmt getQueryStmt() { return modifyImpl_.getSourceStmt(); } nit: could be getSourceStmt, or ModifyImpl's getSourceStmt could be getQueryStmt http://gerrit.cloudera.org:8080/#/c/20477/1/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/20477/1/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@29 PS1, Line 29: import org.apache.impala.planner.TableSink; org.apache.impala.planner.TableSink, org.apache.impala.thrift.TSortingOrder, com.google.common.collect.ImmutableList imports could be removed http://gerrit.cloudera.org:8080/#/c/20477/1/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@78 PS1, Line 78: private void createModifyImpl() { createModifyImpl could be extracted to ModifyStmt as an abstract method -- To view, visit http://gerrit.cloudera.org:8080/20477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If15f64944f2e23064b7112ad5930abc775dd65ec Gerrit-Change-Number: 20477 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Mon, 18 Sep 2023 11:10:50 +0000 Gerrit-HasComments: Yes
