Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18589 )

Change subject: [java] KUDU-2671 support adding a range with custom hash schema
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18589/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java:

http://gerrit.cloudera.org:8080/#/c/18589/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@607
PS8, Line 607: co
> nit: non
Done


http://gerrit.cloudera.org:8080/#/c/18589/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:

http://gerrit.cloudera.org:8080/#/c/18589/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@1715
PS8, Line 1715: fail("should not be able to add a partition which overlaps with 
existing unbounded one");
> Does this mean if a table is initially created without any range partitions
Yes, exactly -- that's how it supposed to work regardless of the bound type 
(unlimited or limited).


http://gerrit.cloudera.org:8080/#/c/18589/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@1872
PS8, Line 1872: assertTrue(buckets.contains(0));
              :         assertTrue(buckets.contains(1));
              :         assertTrue(buckets.contains(2));
              :         assertTrue(buckets.contains(3));
              :         assertTrue(buckets.contains(4));
              :         assertTrue(buckets.contains(5));
              :         assertTrue(buckets.contains(6));
> nit: a loop could be used here, checking buckets.contains(i) where i < buck
Done


http://gerrit.cloudera.org:8080/#/c/18589/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@1906
PS8, Line 1906:  Insert more rows: those should go into the newly added range 
with custom
              :     // hash schema.
> Maybe I'm missing something, but why do these rows go into the new range wi
Ah, the point is to insert more rows, so those go both into the range with the 
table-wide and the custom hash schema ranges.

I updated the comment.



--
To view, visit http://gerrit.cloudera.org:8080/18589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieaab7a79d4336de7ff6ec84b8c1806407e4fa44e
Gerrit-Change-Number: 18589
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Fri, 01 Jul 2022 17:11:39 +0000
Gerrit-HasComments: Yes

Reply via email to