Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17090 )
Change subject: KUDU-2671: Adds compatibility for per range hash schemas with unbounded ranges. ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/17090/2/src/kudu/common/partition.cc File src/kudu/common/partition.cc: http://gerrit.cloudera.org:8080/#/c/17090/2/src/kudu/common/partition.cc@521 PS2, Line 521: if (partition.range_key_start().empty()) { : for (int i = static_cast<int>(partition.hash_buckets().size()) - 1; i >= 0; i--) { : if (partition.hash_buckets()[i] != 0) { : break; : } : partition.partition_key_start_.erase(kEncodedBucketSize * i); : } : } nit: maybe comment "Find the first zero-valued bucket from the end and truncate the partition key starting from that bucket onwards." http://gerrit.cloudera.org:8080/#/c/17090/2/src/kudu/common/partition.cc@531 PS2, Line 531: partition.partition_key_end_.erase(kEncodedBucketSize * i); : int32_t hash_bucket = partition.hash_buckets()[i] + 1; nit: maybe comment """ Starting from the last hash bucket, truncate the partition key until we hit the first non-max-valued bucket, at which point, we replace the encoding with the next-incremented bucket value. For example, the following range end partition keys should be transformed, where the key is (hash_comp1, hash_comp2, range_comp): [ (0, 0, "a2b2"), (0, 1, _) ) -> [ (0, 0, "a2b2"), (0, 1, _) ) [ (0, 1, "a2b2"), (0, 1, _) ) -> [ (0, 1, "a2b2"), (1, _, _) ) [ (1, 0, "a2b2"), (1, 0, _) ) -> [ (1, 0, "a2b2"), (1, 1, _) ) [ (1, 1, "a2b2"), (1, 1, _) ) -> [ (1, 1, "a2b2"), (_, _, _) ) """ http://gerrit.cloudera.org:8080/#/c/17090/2/src/kudu/common/partition.cc@533 PS2, Line 533: HashBucketSchema Use a pointer, otherwise we're making a copies here. http://gerrit.cloudera.org:8080/#/c/17090/2/src/kudu/common/partition.cc@537 PS2, Line 537: partition_mapping[j] nit: how about calling this 'schemas_idx_by_partition_idx' or something? Or defining some const auto& bounds_idx_for_partition = partition_mapping[j]; -- To view, visit http://gerrit.cloudera.org:8080/17090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f6c709e211359b04f7597af5f670c787bda7481 Gerrit-Change-Number: 17090 Gerrit-PatchSet: 2 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 22 Feb 2021 08:22:56 +0000 Gerrit-HasComments: Yes
