Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17890 )
Change subject: WIP KUDU-2671 introduce PartitionKey ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/common/partition.h File src/kudu/common/partition.h: http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/common/partition.h@102 PS3, Line 102: other < *this; nit: switch order to *this > other; Easier to read given the operator is >. http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/common/partition.h@106 PS3, Line 106: return other < *this || other == *this; nit: same as above. http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/common/partition_pruner-test.cc File src/kudu/common/partition_pruner-test.cc: http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/common/partition_pruner-test.cc@1260 PS3, Line 1260: 12 - 5 12 - 5 = 7 so isn't this the result we're expecting? http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/17890/3/src/kudu/master/master.proto@621 PS3, Line 621: message TablePartitionKeyPB { Would it make sense to split up this protobuf into one message just encapsulating one range key and one hash key? message TablePartitionKeyPB { optional bytes range_key = 1 [(kudu.REDACT) = true]; optional bytes hash_key = 2; } Then in GetTableLocationsRequestPB we can introduce two new fields such as: optional TablePartitionKeyPB key_range_start = 8; optional TablePartitionKeyPB key_range_end = 9; -- To view, visit http://gerrit.cloudera.org:8080/17890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00255ec404beeb999117f5265de0d5d8deaf0d68 Gerrit-Change-Number: 17890 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Fri, 01 Oct 2021 05:49:26 +0000 Gerrit-HasComments: Yes
