Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19383 )
Change subject: IMPALA-11809: Support non unique primary key for Kudu ...................................................................... Patch Set 13: (5 comments) I need to do another pass. In the meantime, I have some questions and nits. http://gerrit.cloudera.org:8080/#/c/19383/13//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19383/13//COMMIT_MSG@9 PS13, Line 9: Kudu engine recently enables the auto-incrementing column feature Any KUDU jira number that we can mention here? http://gerrit.cloudera.org:8080/#/c/19383/13//COMMIT_MSG@28 PS13, Line 28: specifying PRIMARY KEY is optional Is this new behavior? Kudu doc says declaring primary key is mandatory https://kudu.apache.org/docs/schema_design.html#primary-keys Kudu doc says "Primary key columns must be non-nullable, and may not be a boolean, float or double type.". Is this also true for NON UNIQUE PRIMARY KEY column? http://gerrit.cloudera.org:8080/#/c/19383/13//COMMIT_MSG@30 PS13, Line 30: columes nit: column(s)? http://gerrit.cloudera.org:8080/#/c/19383/13//COMMIT_MSG@36 PS13, Line 36: CREATE TABLE tbl (i INT NON UNIQUE PRIMARY KEY, s STRING) : PARTITION BY HASH (i) partitions 3 : STORED as KUDU; : CREATE TABLE tbl (i INT, s STRING, NON UNIQUE PRIMARY KEY(i)) : PARTITION BY HASH (i) partitions 3 : STORED as KUDU; : CREATE TABLE tbl NON UNIQUE PRIMARY KEY(id) : PARTITION BY HASH (id) partitions 3 : STORED as KUDU : AS SELECT id, string_col FROM functional.alltypes WHERE id = 10; nit: Add newline between examples? http://gerrit.cloudera.org:8080/#/c/19383/13/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test File testdata/workloads/functional-query/queries/QueryTest/kudu_create.test: http://gerrit.cloudera.org:8080/#/c/19383/13/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test@533 PS13, Line 533: 'Table has been created.' Does it makes sense to give feedback to user that (a, b) are automatically promoted as NON UNIQUE PRIMARY KEY? This can help if user forgot to name PRIMARY KEY, which used to be mandatory before this patch. -- To view, visit http://gerrit.cloudera.org:8080/19383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d7882bf3d01a3492cc9827c072d1f3200d9eebd Gerrit-Change-Number: 19383 Gerrit-PatchSet: 13 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Wed, 01 Feb 2023 23:37:46 +0000 Gerrit-HasComments: Yes
