Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10831 )
Change subject: KUDU-2191: reject alter table rename to same table name ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/10831/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/10831/3/src/kudu/master/catalog_manager.cc@2153 PS3, Line 2153: This : // invariant is also checked in CatalogManager::AlterTable, but when the HMS : // integration is enabled that check does not bubble up to the client (it's : // called by the notification log listener). > > I thought "interesting, does that mean this patch should be reverted when We're in agreement about the necessity of this check; what I'm trying to point out is that, as worded, this comment is confusing: it seems to suggest that the check is necessary because the result of the AlterTable() check doesn't make it back to the client. But as we discussed, you'd want this check here even if the check in AlterTable() propagated all the way to the client. I think the suggestion is more obvious if I make the comment more abstract: "This invariant is also enforced in Foo(), but when condition Bar is true, the enforcement in Foo() is faulty [thus implying that the enforcement here is still necessary]". To continue the example, this reads weird because we actually need the enforcement here regardless of whether the enforcement in Foo() is faulty or not. -- To view, visit http://gerrit.cloudera.org:8080/10831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d895bcc98cbc45a27ff25e97a25436273d79b0 Gerrit-Change-Number: 10831 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Wed, 27 Jun 2018 23:04:54 +0000 Gerrit-HasComments: Yes
