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 18: (2 comments) Thanks Riza. http://gerrit.cloudera.org:8080/#/c/19383/17/fe/src/main/java/org/apache/impala/catalog/KuduColumn.java File fe/src/main/java/org/apache/impala/catalog/KuduColumn.java: http://gerrit.cloudera.org:8080/#/c/19383/17/fe/src/main/java/org/apache/impala/catalog/KuduColumn.java@71 PS17, Line 71: if (isKey) { : Preconditions.checkArgument(!isPrimaryKeyUnique || !isAutoIncrementing); > May I suggest to split this with branch on isKey? Changed Preconditions http://gerrit.cloudera.org:8080/#/c/19383/16/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/16/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@391 PS16, Line 391: isPrimaryKeyUnique_ = kuduSchema_.isPrimaryKeyUnique(); : hasAutoIncrementingColumn_ = kuduSchema_.hasAutoIncrementingColumn(); > So is it valid to add this precondition here? Yes, added precondition -- 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: 18 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: Sat, 04 Feb 2023 02:03:06 +0000 Gerrit-HasComments: Yes
