Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9965 )

Change subject: KUDU-2191 (9/n): HMS Catalog should short-circuit no-op alter 
and create table calls
......................................................................


Patch Set 3: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9965/2/src/kudu/hms/hms_catalog.cc
File src/kudu/hms/hms_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/9965/2/src/kudu/hms/hms_catalog.cc@218
PS2, Line 218: renamed
> We are now handling 'ALTER TABLE ...' and 'ALTER TABLE RENAME  ...' operati
Makes sense; I missed the L208 short-circuit and thought these code paths were 
for all ALTERs.


http://gerrit.cloudera.org:8080/#/c/9965/2/src/kudu/hms/hms_catalog.cc@222
PS2, Line 222:         return CreateOrUpdateTable(client, id, new_name, schema, 
master_addresses_);
> No, this is actually the critical change of the patch; previously we weren'
Yes, I figured it was important to call CreateOrUpdateTable(), was just 
wondering whether _some_ of the work it did was duplicated.

On second look though, there is a key difference between the calls to 
ParseTableName() and GetTable() here and in CreateOrUpdateTable(): the calls 
here use the old table name while the calls in CreateOrUpdateTable() use the 
new table name. So they are completely different and there's no duplicate work.


http://gerrit.cloudera.org:8080/#/c/9965/2/src/kudu/hms/hms_client.cc
File src/kudu/hms/hms_client.cc:

http://gerrit.cloudera.org:8080/#/c/9965/2/src/kudu/hms/hms_client.cc@119
PS2, Line 119: const char* const HmsClient::kStorageHandlerKey = 
"storage_handler";
> The motivation to re-define it is that the name of the constant is really w
I don't feel strongly, I just wanted to know whether this was a cosmetic thing 
as I suspected it was, or whether there was a different reason.



--
To view, visit http://gerrit.cloudera.org:8080/9965
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c3594db0db253bc88e032b8c5aa2c8667c49853
Gerrit-Change-Number: 9965
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 10 Apr 2018 18:23:44 +0000
Gerrit-HasComments: Yes

Reply via email to