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

Reply via email to