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

Change subject: WIP [proto] KUDU-2671 update range partitioning with customs 
hash schema
......................................................................


Patch Set 1:

(4 comments)

Sorry for the late review. In regards to storing the hash dimension info in 
PartitionPB, that would be superfluous. Imagine a range with a plethora of 
partitions, each partition would store the same hash dimension rather than just 
the range bounds and its hash dimension being stored once within 
PartitionSchemaPB.

http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@358
PS1, Line 358: uint32
If we're changing this to be unsigned, would have to make similar changes 
elsewhere in codebase.


http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@367
PS1, Line 367:  message RangeWithHashSchemaPB {
             :     // A set of row operations containing the lower and upper 
range bound
             :     // for the range.
             :     optional RowOperationsPB range_bounds = 1;
             :     repeated HashDimensionPB hash_schema = 2;
             :   }
             :
+1

This a neater approach than having two separate fields and ensuring they have 
the same size.


http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@374
PS1, Line 374: repeated HashBucketSchemaPB hash_schema = 1;
Eventually this will be deprecated correct?


http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@377
PS1, Line 377: //repeated PerRangeHashBucketSchemasPB range_hash_schemas = 3;
             :   //repeated RowOperationsPB range_bounds = 4;
Should we mark these as deprecated?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37aae56a33170894f30d6cd73a5698d6cbb7a697
Gerrit-Change-Number: 17779
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Comment-Date: Wed, 18 Aug 2021 22:14:32 +0000
Gerrit-HasComments: Yes

Reply via email to