[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/18953 )
Change subject: IMPALA-11565: Support IF NOT EXISTS in alter table add columns for kudu/iceberg table ...................................................................... Patch Set 12: (3 comments) Thanks for working on this! I left some minor comments here. http://gerrit.cloudera.org:8080/#/c/18953/12/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/18953/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1343 PS12, Line 1343: params.getAdd_cols_params() != null : && params.getAdd_cols_params().getColumnsSize() != 0 A lot of similar logic code, can extract into a method? http://gerrit.cloudera.org:8080/#/c/18953/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test: http://gerrit.cloudera.org:8080/#/c/18953/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test@439 PS12, Line 439: ---- QUERY What happens when we add column for the same name, but of a different type, if they already exist? http://gerrit.cloudera.org:8080/#/c/18953/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test@442 PS12, Line 442: alter table ice_alter_cols add if not exists columns (a bigint, d bigint) nit: The case of sql should be as consistent as possible with the rest of the code in the current file. -- To view, visit http://gerrit.cloudera.org:8080/18953 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82590e5372e881f2e81d4ed3dd0d32a2d3ddb517 Gerrit-Change-Number: 18953 Gerrit-PatchSet: 12 Gerrit-Owner: Baike Xia <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Baike Xia <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jian Zhang <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 27 Dec 2022 03:17:22 +0000 Gerrit-HasComments: Yes
