Mahesh Reddy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17769 )

Change subject: KUDU-2671 key encoding for custom hash bucket schemas
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/17769/6/src/kudu/client/client.cc
File src/kudu/client/client.cc:

http://gerrit.cloudera.org:8080/#/c/17769/6/src/kudu/client/client.cc@1016
PS6, Line 1016: if (range->hash_bucket_schemas_.empty()) {
              :         schemas_pb->mutable_hash_schemas()->CopyFrom(
              :             data_->partition_schema_.hash_bucket_schemas())
> Right: eventually there should be no table-wide hash schema stored.  There
The only issue with this approach is that the user needs to manually add a hash 
dimension for each range, even if the same hash dimension is to be used for 
each range or most of the ranges. That would put more of an onus on the user.


http://gerrit.cloudera.org:8080/#/c/17769/6/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:

http://gerrit.cloudera.org:8080/#/c/17769/6/src/kudu/common/partition.cc@959
PS6, Line 959: if (key.size() <  kEncodedBucketSize * 
hash_bucket_schemas_.size()) {
             :       return "<hash-decode-error>";
             :     }
> Good point!  I guess this part should have been moved down right before the
Ack


http://gerrit.cloudera.org:8080/#/c/17769/6/src/kudu/common/partition.cc@1558
PS6, Line 1558: // TODO(aserbin): is the upper bound always exclusive?
> Thank you for the confirmation -- I removed this comment in https://gerrit.
Ack


http://gerrit.cloudera.org:8080/#/c/17769/6/src/kudu/common/partition.cc@1559
PS6, Line 1559: if (!upper.empty() && upper <= range_key) {
              :       has_custom_range = false;
              :     }
> This statement is triggered in a case of non-covered range: FindFloorOrNull
Makes sense, when we get rid of the renamed field 'hash_schema_', what will be 
returned if this if condition is met?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I81aa1c10998d88a1bd5314fc3132037b1c8bbe4b
Gerrit-Change-Number: 17769
Gerrit-PatchSet: 6
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, 18 Aug 2021 08:41:35 +0000
Gerrit-HasComments: Yes

Reply via email to