Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16859 )
Change subject: [master] Range specific hashing at table creation time. ...................................................................... Patch Set 2: (1 comment) > Patch Set 2: > > (1 comment) 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 (!split_rows.empty()) { : for (const auto& hash_schemas : range_hash_schemas) { : if (!hash_schemas.empty()) { : return Status::InvalidArgument("Both 'split_rows' and 'range_hash_schemas' cannot be " : "populated at the same time."); : } : } > > Currently, the size of 'range_hash_schemas' matches 'range_bounds' throug I meant to say if the user was using the new client API, the size of 'range_hash_schemas' would match the size of 'range_bounds'. If we decide that the user can ONLY use the old API or ONLY use the new API, then I agree with your two assumptions. However, I've been operating under the assumption that the user can use both API's simultaneously. I've been trying to cover the case where both 'split_rows' and 'range_bounds' are defined. As for your last point, if both 'split_rows' and 'range_bounds' are defined, and 'range_hash_schema' was a vector of empty vectors, then the same default table wide schema would be applied to each bound so there's no ambiguity to 'split_rows' being defined. Ultimately, I will say the implementation would be a lot easier and more readable if we decided that only the two cases you stated were possible, but if we continue the current way this patch is written then more flexibility can be given to the user at the cost of some code readability. -- 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: 2 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: Tue, 15 Dec 2020 02:52:50 +0000 Gerrit-HasComments: Yes
