Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19383 )
Change subject: WIP IMPALA-11809: Support non unique primary key for Kudu ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/19383/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java: http://gerrit.cloudera.org:8080/#/c/19383/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@107 PS8, Line 107: A primary key is required for a Kudu table Is this the right Error message for this case? Additionally, if a user wants to create multiple partition levels, the columns on which the partitions are being created have to be specified first, right? In that case does the order matter as long as all the columns are the beginning of the table? http://gerrit.cloudera.org:8080/#/c/19383/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@715 PS8, Line 715: add Do we need an equivalent test for dropping of the columns? http://gerrit.cloudera.org:8080/#/c/19383/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: http://gerrit.cloudera.org:8080/#/c/19383/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2798 PS8, Line 2798: i INT PRIMARY KEY, NON UNIQUE PRIMARY KEY(i) This looks a bit confusing for the end user where is declared as both PRIMARY KEY and NON UNIQUE PRIMARY KEY. Do we want to allow this? Additionally what if we do something like below? What would the result be? i INT NON UNIQUE PRIMARY KEY, PRIMARY KEY(i) http://gerrit.cloudera.org:8080/#/c/19383/8/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/8/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test@535 PS8, Line 535: A primary key is required for a Kudu table As pointed out before, maybe a more helpful message might help here? -- 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: 8 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Marton Greber <greber...@gmail.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Tue, 10 Jan 2023 19:25:02 +0000 Gerrit-HasComments: Yes