Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12181 )
Change subject: IMPALA-7832: Support for IF NOT EXISTS in ALTER TABLE ADD COLUMN(S) ...................................................................... Patch Set 7: Code-Review+1 (5 comments) Nice test coverage. Paul, looks like you had some comments about refactoring. Can you take the final pass? http://gerrit.cloudera.org:8080/#/c/12181/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/12181/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@612 PS7, Line 612: Column nit: Column(s) have.., here and below. http://gerrit.cloudera.org:8080/#/c/12181/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2046 PS7, Line 2046: nonExistingColumns nit: I think colsToAdd will be more clear. Please ignore if you disagree. http://gerrit.cloudera.org:8080/#/c/12181/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2049 PS7, Line 2049: if (ifNotExists && col != null) { : continue; : } single line and else if below? http://gerrit.cloudera.org:8080/#/c/12181/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2053 PS7, Line 2053: AnalysisException We are past analysis at this point. Throw a CatalogException? Also, include the tbl name? http://gerrit.cloudera.org:8080/#/c/12181/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2057 PS7, Line 2057: and ifNotExists is true, do nothing we don't use it anymore, update? -- To view, visit http://gerrit.cloudera.org:8080/12181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I60ed22c8a8eefa10e94ad3dedf32fe67c16642d9 Gerrit-Change-Number: 12181 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Greg Rahn <gr...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com> Gerrit-Comment-Date: Fri, 18 Jan 2019 19:41:34 +0000 Gerrit-HasComments: Yes
