Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18515 )
Change subject: [master] KUDU-2671: Range specific hashing during table alter op. ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/18515/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/18515/4/src/kudu/master/catalog_manager.cc@2609 PS4, Line 2609: PartitionSchema partition_schema; : RETURN_NOT_OK(PartitionSchema::FromPB( : l.data().pb.partition_schema(), schema, &partition_schema)); Maybe this can be a different patch, but the new field 'ranges_with_hash_schemas' of PartitionSchema will have to be updated to account for the new set of ranges and their respective hash schema. http://gerrit.cloudera.org:8080/#/c/18515/4/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/18515/4/src/kudu/master/master-test.cc@985 PS4, Line 985: ; not sure if this only on gerrit, but move semicolon to be directly after end parantheses without the line break. http://gerrit.cloudera.org:8080/#/c/18515/4/src/kudu/master/master-test.cc@1011 PS4, Line 1011: ASSERT_STR_CONTAINS(resp.error().status().DebugString(), : "varying number of hash dimensions per range is not yet supported"); Can you add another test case where a range partition with a custom hash schema is added successfully (has the same number of hash dimensions as the table wide hash schema)? Just looking for more test coverage. -- To view, visit http://gerrit.cloudera.org:8080/18515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iea9e3317d172c9ae76662c44b21fca9a4819930a Gerrit-Change-Number: 18515 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Tue, 24 May 2022 20:37:13 +0000 Gerrit-HasComments: Yes
