Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16859 )
Change subject: [master] KUDU-2671: Range specific hashing at table creation time. ...................................................................... Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/16859/10/src/kudu/common/partition-test.cc File src/kudu/common/partition-test.cc: http://gerrit.cloudera.org:8080/#/c/16859/10/src/kudu/common/partition-test.cc@1148 PS10, Line 1148: (splits, bounds, {}, Now that there's no per-range hash partitioning in this test, does this test add anything given TestCompoundRangeKeyEncoding exists? More tests is generally good, but preferably only if adds test coverage relevant to the patch at hand. http://gerrit.cloudera.org:8080/#/c/16859/2/src/kudu/common/partition.cc File src/kudu/common/partition.cc: http://gerrit.cloudera.org:8080/#/c/16859/2/src/kudu/common/partition.cc@418 PS2, Line 418: if (!range_hash_schemas.empty()) { : if (!split_rows.empty()) { : return Status::InvalidArgument("Both 'split_rows' and 'range_hash_schemas' cannot be " : "populated at the same time."); : } : if (range_bounds.size() != range_hash_schemas.size()) { : > After much thought, I've decided that the best route is what you said above SGTM, thanks for the update. -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 10 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Mon, 04 Jan 2021 07:20:42 +0000 Gerrit-HasComments: Yes
