Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15841 )
Change subject: KUDU-3090: Native owner metadata in Kudu ...................................................................... Patch Set 12: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/15841/12/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/15841/12/src/kudu/integration-tests/master_hms-itest.cc@303 PS12, Line 303: TEST_F(MasterHmsTest, TestAlterTableOwner) { Would it also be possible to test the case where we're adding an owner to a table that doesn't have an existing owner? E.g. if we're upgrading from an older version of Kudu and decide to add an owner in Kudu. The empty string case seems similar, but I think they're slightly different. http://gerrit.cloudera.org:8080/#/c/15841/12/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/15841/12/src/kudu/master/catalog_manager.cc@187 PS12, Line 187: n") nit: unrelated, but could you add a period here? http://gerrit.cloudera.org:8080/#/c/15841/12/src/kudu/master/catalog_manager.cc@2684 PS12, Line 2684: const string& owner = l.mutable_data()->pb.owner(); nit: could you move this closer to where it's used so readers don't need look around for its usage site? -- To view, visit http://gerrit.cloudera.org:8080/15841 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67f5bfdf56d409960365fd5803913a2d3800831d Gerrit-Change-Number: 15841 Gerrit-PatchSet: 12 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Jul 2020 01:48:03 +0000 Gerrit-HasComments: Yes
