Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19272 )
Change subject: Auto-Incrementing Column, C++ client part1 ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/client.h@3226 PS1, Line 3226: DeserializeIntoScanner( If auto_increment_id column is added in token, the scanner have to read auto_increment_id column. http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema-internal.h File src/kudu/client/schema-internal.h: http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema-internal.h@89 PS1, Line 89: non_unique_primary_key; non_unique_primary_key have conflict with primary_key. How about change non_unique_primary_key to non_unique_key? For unique primary key, primary_key is set as true, non_unique_key is set as false. For non unique primary key, primary_key and non_unique_key are set as true. http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema.h@497 PS1, Line 497: SetNonPrimaryKey nit: SetNonUniquePrimaryKey() http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema.h@502 PS1, Line 502: NonUniquePrimaryKey Two sets of functions PrimaryKey() and NonUniquePrimaryKey() are defined for unique keys and non unique keys. We have to check if both functions are called. How about combining two functions as one, change PrimaryKey() as "KuduColumnSpec* PrimaryKey(bool is_non_unique = false)"? http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema.h@643 PS1, Line 643: SetNonUniquePrimaryKey How about combining this functions with SetPrimaryKey(), change SetPrimaryKey() as "KuduSchemaBuilder* SetPrimaryKey(const std::vector<std::string>& key_col_names, bool is_non_unique = false)"? http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/client/schema.h@755 PS1, Line 755: need API to to check if the primary key is unique or not, and API to get auto_increment_id index. http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/common/schema.cc File src/kudu/common/schema.cc: http://gerrit.cloudera.org:8080/#/c/19272/1/src/kudu/common/schema.cc@198 PS1, Line 198: const std::string ColumnSchema::auto_incrementing_id = "auto_incrementing_id"; Could we define the column name of auto_incrementing_id as constant and put it in client.h? For CTAS, Impala planner create a temporary KuduTable for insertion. Impala has to simulate Kudu engine to add auto_incrementing_id column in the temporary KuduTable for non unique primary key. We need to use same column name. -- To view, visit http://gerrit.cloudera.org:8080/19272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic133e3d44cc56c8351e33d95b523ed7b6b13617b Gerrit-Change-Number: 19272 Gerrit-PatchSet: 1 Gerrit-Owner: Marton Greber <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Thu, 24 Nov 2022 10:10:34 +0000 Gerrit-HasComments: Yes
