Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16596 )
Change subject: WIP [partitioning] KUDU-2671: Support for range specific HashSchemas. ...................................................................... Patch Set 4: (4 comments) > Patch Set 4: > > (4 comments) http://gerrit.cloudera.org:8080/#/c/16596/4/src/kudu/common/partition.cc File src/kudu/common/partition.cc: http://gerrit.cloudera.org:8080/#/c/16596/4/src/kudu/common/partition.cc@276 PS4, Line 276: range_partitions_with_hash_schemas > nit: add four spaces here since it's a continuation of the previous line Done http://gerrit.cloudera.org:8080/#/c/16596/4/src/kudu/common/partition.cc@295 PS4, Line 295: range_hash_schema[j++] > What if this is empty? oversight on my part, will have insert an if check for now. http://gerrit.cloudera.org:8080/#/c/16596/4/src/kudu/common/partition.cc@302 PS4, Line 302: &t1 > nit: put the & with the rest of the type; same below Done http://gerrit.cloudera.org:8080/#/c/16596/4/src/kudu/common/partition.cc@418 PS4, Line 418: RETURN_NOT_OK(EncodeRangeBounds(range_bounds, range_hash_schema, schema, &bounds, : &bounds_with_hash_schemas)); : RETURN_NOT_OK(EncodeRangeSplits(split_rows, schema, &splits)); : RETURN_NOT_OK(SplitRangeBounds(schema, std::move(splits), &bounds)); > We chatted on Slack about maybe using a struct{string,string,optional<HashB More code would have to be changed if I just use a single struct instead of duplicating 'bounds', but the changes would be minor and ultimately I agree avoiding this duplication is optimal. -- To view, visit http://gerrit.cloudera.org:8080/16596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8725f4bd072a81b05b36dfc7df0c074c172b4ce8 Gerrit-Change-Number: 16596 Gerrit-PatchSet: 4 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 19 Nov 2020 06:09:48 +0000 Gerrit-HasComments: Yes
