Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/21486 )
Change subject: KUDU-3577 fix altering tables with custom hash schemas ...................................................................... Patch Set 2: Code-Review+1 (3 comments) LGTM. Just a couple of nits. http://gerrit.cloudera.org:8080/#/c/21486/2/src/kudu/integration-tests/alter_table-test.cc File src/kudu/integration-tests/alter_table-test.cc: http://gerrit.cloudera.org:8080/#/c/21486/2/src/kudu/integration-tests/alter_table-test.cc@2232 PS2, Line 2232: { nit: Add a comment about dropping a column in this code block. http://gerrit.cloudera.org:8080/#/c/21486/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/21486/2/src/kudu/master/catalog_manager.cc@3031 PS2, Line 3031: !current_pb.partition_schema().custom_hash_schema_ranges().empty() && Just curious: Would this be applicable to cases other than DROP_COLUMN? If not, would it make sense to move this block under applicable case only, to avoid unnecessary checks? http://gerrit.cloudera.org:8080/#/c/21486/2/src/kudu/master/catalog_manager.cc@3782 PS2, Line 3782: nit: remove whitespaces -- To view, visit http://gerrit.cloudera.org:8080/21486 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21a775538063768b986edd2b6bc25d03360b5216 Gerrit-Change-Number: 21486 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> Gerrit-Comment-Date: Fri, 07 Jun 2024 12:20:05 +0000 Gerrit-HasComments: Yes
