You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/05/20 22:18:02 UTC
[kudu-CR] hms: syncronize column comments to the HMS
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13381
Change subject: hms: syncronize column comments to the HMS
......................................................................
hms: syncronize column comments to the HMS
This patch ensures column comments are
syncronized to the HMS when a table is created
or altered.
Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/13381/1
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: hms: synchronize column comments to the HMS
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13381
to look at the new patch set (#3).
Change subject: hms: synchronize column comments to the HMS
......................................................................
hms: synchronize column comments to the HMS
This patch ensures column comments are
synchronized to the HMS when a table is created
or altered.
Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 12 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/13381/3
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13381 )
Change subject: hms: synchronize column comments to the HMS
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 May 2019 16:34:26 +0000
Gerrit-HasComments: No
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13381 )
Change subject: hms: synchronize column comments to the HMS
......................................................................
hms: synchronize column comments to the HMS
This patch ensures column comments are
synchronized to the HMS when a table is created
or altered.
Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Reviewed-on: http://gerrit.cloudera.org:8080/13381
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Grant Henke <gr...@apache.org>
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 12 insertions(+), 1 deletion(-)
Approvals:
Hao Hao: Looks good to me, approved
Grant Henke: Verified
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13381 )
Change subject: hms: synchronize column comments to the HMS
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13381/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13381/1//COMMIT_MSG@7
PS1, Line 7: syncronize
> synchronize
Done
http://gerrit.cloudera.org:8080/#/c/13381/1//COMMIT_MSG@10
PS1, Line 10: syncronized
> synchronized
Done
http://gerrit.cloudera.org:8080/#/c/13381/1/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13381/1/src/kudu/integration-tests/master_hms-itest.cc@245
PS1, Line 245: comment_alterer->AlterColumn("key")->Comment("Sample Comment");
> Does it make sense to add a scenario that verifies that setting the comment empty (or otherwise clearing the comment) reflects in the HMS as well? Or we have some other means to make sure this corner case works as expected?
Sure I can do this.
> Also, what about a scenario when a new column is being added with a comment?
There is nothing unique about this case code wise, so testing feels a bit overkill.
> And is it going to work for non-key columns as well?
The HMS has no concept of key columns, this works the same for all columns.
> What if a column is renamed -- should the comment transparently appear in HMS as an attribute of the renamed column?
Yes, column renames have no interaction or impact on this.
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 May 2019 15:05:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13381 )
Change subject: hms: synchronize column comments to the HMS
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 May 2019 16:47:17 +0000
Gerrit-HasComments: No
[kudu-CR] hms: synchronize column comments to the HMS
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13381
to look at the new patch set (#2).
Change subject: hms: synchronize column comments to the HMS
......................................................................
hms: synchronize column comments to the HMS
This patch ensures column comments are
synchronized to the HMS when a table is created
or altered.
Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 11 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/13381/2
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] hms: syncronize column comments to the HMS
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13381 )
Change subject: hms: syncronize column comments to the HMS
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13381/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13381/1//COMMIT_MSG@7
PS1, Line 7: syncronize
synchronize
http://gerrit.cloudera.org:8080/#/c/13381/1//COMMIT_MSG@10
PS1, Line 10: syncronized
synchronized
http://gerrit.cloudera.org:8080/#/c/13381/1/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13381/1/src/kudu/integration-tests/master_hms-itest.cc@245
PS1, Line 245: comment_alterer->AlterColumn("key")->Comment("Sample Comment");
Does it make sense to add a scenario that verifies that setting the comment empty (or otherwise clearing the comment) reflects in the HMS as well? Or we have some other means to make sure this corner case works as expected?
Also, what about a scenario when a new column is being added with a comment?
And is it going to work for non-key columns as well?
What if a column is renamed -- should the comment transparently appear in HMS as an attribute of the renamed column?
--
To view, visit http://gerrit.cloudera.org:8080/13381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12da391ea74e153483b6657e7028aa6784ac41b3
Gerrit-Change-Number: 13381
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 20 May 2019 22:49:57 +0000
Gerrit-HasComments: Yes