Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17825 )
Change subject: KUDU-2671 allow a range to have no hash schema ...................................................................... Patch Set 4: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/17825/4/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/17825/4/src/kudu/client/client.h@1251 PS4, Line 1251: Besides, nit: "Besides" is a bit informal. Perhaps use "Alternatively"? http://gerrit.cloudera.org:8080/#/c/17825/4/src/kudu/client/client.h@1312 PS4, Line 1312: the table-wide hash : /// partitioning schema can be set using the : /// @c KuduTableCreator::add_hash_partitions() method nit: it seems a bit odd to mention the table-wide hash schema here, given this method _doesn't_ use the table-wide hash schema, and that KuduTableCreator::add_hash_partitions() isn't particularly relevant to this method in general. Maybe instead, mention the intricacies of add_custom_range_partition(), e.g. - When called with a KuduRangePartition for which KuduRangePartition::add_hash_partitions() hasn't been called, a range with no hash sub-partitioning is created. - To create a range with the table-wide hash schema, use KuduTableCreator::add_range_partition() instead. I suppose both of these can be inferred from the comments elsewhere, but it seems nice to have these close to this method in particular, given I suspect folks will look at the table creator APIs before looking at the range partition API. http://gerrit.cloudera.org:8080/#/c/17825/2/src/kudu/client/flex_partitioning_client-test.cc File src/kudu/client/flex_partitioning_client-test.cc: http://gerrit.cloudera.org:8080/#/c/17825/2/src/kudu/client/flex_partitioning_client-test.cc@412 PS2, Line 412: tor::add_hash_part > add_custom_range_partition() is a method of KuduTableCreator; here the idea Ah, indeed. I misinterpreted this as, "calling add_custom_range_partition() without adding additional hash partitions means the range doesn't have any hash bucketing". -- To view, visit http://gerrit.cloudera.org:8080/17825 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia43df69ecd7040e285e098fde49d84a7a00d1fbb Gerrit-Change-Number: 17825 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Wed, 08 Sep 2021 21:23:26 +0000 Gerrit-HasComments: Yes
