Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14592 )
Change subject: IMPALA-2112: Support primary key/foreign key constraints as part of create table in Impala. ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/14592/3/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/14592/3/common/thrift/CatalogObjects.thrift@370 PS3, Line 370: 8: optional list<hive_metastore.SQLPrimaryKey> primary_keys, > In line 353, there's a removed field id 8. However, I think it's ok since T Do you think it okay to use 8? If not, I will change it to use new unused number. http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@28 PS4, Line 28: import org.apache.impala.authorization.AuthorizationConfig; > nit: import lines are sorted. It'd be better to not break it. Did not observe that they are sorted. Thanks for letting me know. http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@50 PS4, Line 50: import com.google.common.primitives.Ints; > nit: import lines are sorted. Done http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/analysis/TableDef.java File fe/src/main/java/org/apache/impala/analysis/TableDef.java: http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/analysis/TableDef.java@461 PS4, Line 461: } else { : prim > nit: one line Done http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@233 PS4, Line 233: // > nit: need space Done http://gerrit.cloudera.org:8080/#/c/14592/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@958 PS4, Line 958: //G > nit: need space Done -- To view, visit http://gerrit.cloudera.org:8080/14592 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id03d8d4d41a2ac1b15e7060e2a013e334d044ee7 Gerrit-Change-Number: 14592 Gerrit-PatchSet: 5 Gerrit-Owner: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Thu, 07 Nov 2019 02:35:45 +0000 Gerrit-HasComments: Yes
