Wenzhe Zhou 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 16:

(7 comments)

Thanks Qifan for your review.

http://gerrit.cloudera.org:8080/#/c/19383/15//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19383/15//COMMIT_MSG@23
PS15, Line 23: within a continuous region of the table served by a 
tablet-server.
> nit within a continuous region of the table served by a tablet-server.
Done


http://gerrit.cloudera.org:8080/#/c/19383/15//COMMIT_MSG@25
PS15, Line 25: a
> nit also
Done


http://gerrit.cloudera.org:8080/#/c/19383/15//COMMIT_MSG@47
PS15, Line 47:   AS SELECT id, string_col FROM functional.alltypes WHERE id = 
10;
> It will be great to include a DDL for a range partitioned kudu table here,
Done


http://gerrit.cloudera.org:8080/#/c/19383/15//COMMIT_MSG@57
PS15, Line 57:
> Kudu
Done


http://gerrit.cloudera.org:8080/#/c/19383/15//COMMIT_MSG@59
PS15, Line 59:  (id
> nit. for these tables
Done


http://gerrit.cloudera.org:8080/#/c/19383/15/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
File fe/src/main/java/org/apache/impala/catalog/KuduTable.java:

http://gerrit.cloudera.org:8080/#/c/19383/15/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@107
PS15, Line 107:
              :   // Set to true if primary key is unique.
              :   private boolean isPrimaryKeyUnique_ = true;
              :
              :   // Set to true if the table has auto-incrementing column.
              :   private boolean hasAutoIncrementingColumn_ = false;
> These two new members could be put into a new class and reused between Kudu
KuduTable extends from Table, LocalKuduTable extends from LocalTable. They have 
different parent classes. Other two variables primaryKeyColumnNames_ and 
partitionBy_ are also defined in both classes. We could define a new class with 
these 4 variables, but it causes more code change.


http://gerrit.cloudera.org:8080/#/c/19383/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:

http://gerrit.cloudera.org:8080/#/c/19383/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2365
PS10, Line 2365: partition by hash
> To observe auto column works with a range partitioned kudu table such as
Already added similar test case with range partitions in end-to-end 
kudu_create.test.



--
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: 16
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: Thu, 02 Feb 2023 20:07:43 +0000
Gerrit-HasComments: Yes

Reply via email to