Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17775 )
Change subject: [partition] update naming of related entities ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/17775/4/src/kudu/client/table_creator-internal.h File src/kudu/client/table_creator-internal.h: http://gerrit.cloudera.org:8080/#/c/17775/4/src/kudu/client/table_creator-internal.h@50 PS4, Line 50: const uint32_t num_buckets; : const uint32_t seed; To be consistent with 'HashBucketSchemaPB', shouldn't this be const int32_t num_buckets; const uint32_t seed; http://gerrit.cloudera.org:8080/#/c/17775/4/src/kudu/client/table_creator-internal.h@102 PS4, Line 102: int32_t seed) Should be unsigned. http://gerrit.cloudera.org:8080/#/c/17775/1/src/kudu/common/partition.h File src/kudu/common/partition.h: http://gerrit.cloudera.org:8080/#/c/17775/1/src/kudu/common/partition.h@173 PS1, Line 173: : this struct > That's an option, yes. But from the other side, if that data comes from a I guess as long as the 'ranges_with_hash_schemas' field is persisted within the protobuf message then whenever the 'PartitionSchemaPB' the dict will be constructed. Right now, no modifications to the field 'ranges_with_hash_schemas' are made besides when it's populated. Once the alter range partitions work is underway, we should be cognizant of any modifications to the field and ensure any changes are reflected within the dict. http://gerrit.cloudera.org:8080/#/c/17775/1/src/kudu/common/partition.h@175 PS1, Line 175: s > Why do you think static_cast<uint64_t> is necessary there? I was under imp Ah you're right, it would be promoted to an unsigned value. http://gerrit.cloudera.org:8080/#/c/17775/4/src/kudu/common/partition.cc File src/kudu/common/partition.cc: http://gerrit.cloudera.org:8080/#/c/17775/4/src/kudu/common/partition.cc@325 PS4, Line 325: int32_t change to 'uint32_t' http://gerrit.cloudera.org:8080/#/c/17775/4/src/kudu/common/partition.cc@698 PS4, Line 698: int32_t change to 'uint32_t' -- To view, visit http://gerrit.cloudera.org:8080/17775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a858e97090930b21e9c767dac2f5cc8b9816033 Gerrit-Change-Number: 17775 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 17 Aug 2021 23:23:36 +0000 Gerrit-HasComments: Yes
