[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: - kudu table delete_column The alter column tools can be used as: - kudu table column_set_default ( should be provided as a JSON array, e.g. [1] or ["foo"]) - kudu table column_remove_default - kudu table column_set_compression - kudu table column_set_encoding - kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Reviewed-on: http://gerrit.cloudera.org:8080/13976 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo Reviewed-by: Andrew Wong Reviewed-by: Yingchun Lai <405403...@qq.com> --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 534 insertions(+), 3 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, but someone else must approve Andrew Wong: Looks good to me, approved Yingchun Lai: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 13 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 12: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 12 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 20 Aug 2019 01:38:12 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 12: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc@3449 PS11, Line 3449: > I deleted the test case and would like to commit another patch to make smal That sounds reasonable to me. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 12 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 19 Aug 2019 19:28:44 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 12: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 12 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 19 Aug 2019 19:00:17 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc@3449 PS11, Line 3449: > Yes, the type of value is not double and ExtractDouble returns Status::Inva I deleted the test case and would like to commit another patch to make small improvements for JsonReader. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 12 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 19 Aug 2019 10:27:36 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#12). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: - kudu table delete_column The alter column tools can be used as: - kudu table column_set_default ( should be provided as a JSON array, e.g. [1] or ["foo"]) - kudu table column_remove_default - kudu table column_set_compression - kudu table column_set_encoding - kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 534 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/12 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 12 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc@3449 PS11, Line 3449: NO_FATALS(check_bad_input(kFloatColumn, "[123]", "unable to parse")); > This case is a surprising to me. Is this because the JsonReader thinks it's Yes, the type of value is not double and ExtractDouble returns Status::InvalidArgument(). Since an int value could be losslessly converted to a double value, maybe int/uint should also be parsed in ExtracDouble of JsonReader? I think parsing both int and double for Double/Float column is a bit rebundant. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 19 Aug 2019 09:50:36 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 19 Aug 2019 08:15:52 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 19 Aug 2019 04:21:00 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: (1 comment) Overall looks good, but I've got a question about one of the test cases. http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/11/src/kudu/tools/kudu-tool-test.cc@3449 PS11, Line 3449: NO_FATALS(check_bad_input(kFloatColumn, "[123]", "unable to parse")); This case is a surprising to me. Is this because the JsonReader thinks it's an int? If so, should we try parsing the int? -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Sun, 18 Aug 2019 16:31:35 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has removed a vote on this change. Change subject: [tools] Add table tools to delete column and alter column .. Removed Verified+1 by Andrew Wong -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: Verified+1 Failures were caused by NTP issues. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Sun, 18 Aug 2019 15:53:54 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Sun, 18 Aug 2019 15:54:03 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has removed a vote on this change. Change subject: [tools] Add table tools to delete column and alter column .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/10/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/10/src/kudu/tools/kudu-tool-test.cc@3462 PS10, Line 3462: NO_FATALS(check_bad_input( > Nit: indent Done -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Sat, 17 Aug 2019 07:56:20 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#11). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: - kudu table column_set_default ( should be provided as a JSON array, e.g. [1] or ["foo"]) - kudu table column_remove_default - kudu table column_set_compression - kudu table column_set_encoding - kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 535 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/11 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 11 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 10: Code-Review+1 Please ask Andrew to re-review and +2. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 10 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Fri, 16 Aug 2019 19:31:49 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/10/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/10/src/kudu/tools/kudu-tool-test.cc@3462 PS10, Line 3462:NO_FATALS(check_bad_input( Nit: indent -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 10 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Fri, 16 Aug 2019 16:51:18 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 10:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@698
PS6, Line 698: RETURN_NOT_OK_PREPEND(
> Just to clarify: with the new JSON encoding, empty strings are treated as v
Yes. I have added a few tests.
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@173
PS9, Line 173: enum PartitionAction {
> We try to avoid declaring global non-POD-type variables. They're not so muc
Done
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@706
PS9, Line 706: reader.ExtractBool(values[0], /*field=*/nullptr,
_value), msg);
: *value = KuduValue::FromBool(bool_value);
> Maybe you can declare this up front so you needn't duplicate it in multiple
Done
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@767
PS9, Line 767: }
> Could you add a comment here explaining that we're not actually interested
Done
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@797
PS9, Line 797:
: Status ColumnSetEncoding(const RunnerCon
> Should be able to use one of the FindOr* functions from gutil/map-util.h he
Done
--
To view, visit http://gerrit.cloudera.org:8080/13976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Fri, 16 Aug 2019 11:46:19 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#10). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: - kudu table column_set_default ( should be provided as a JSON array, e.g. [1] or ["foo"]) - kudu table column_remove_default - kudu table column_set_compression - kudu table column_set_encoding - kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 533 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/10 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 10 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 9:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@698
PS6, Line 698: case KuduColumnSchema::DataType::INT8:
> If users want to set default value of a STRING/BINARY column to an empty st
Just to clarify: with the new JSON encoding, empty strings are treated as valid
default values just like any other string, right? Could you verify that with a
unit test?
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@173
PS9, Line 173: const unordered_map kCompressionTypeMap {
We try to avoid declaring global non-POD-type variables. They're not so much an
issue in executables, but in libraries they affect loading performance and
their initialization order is undefined (which is a problem when one global
depends on another).
Instead, declare these as static variables within whichever function needs them.
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@706
PS9, Line 706: Substitute("unable to parse value for column type $0",
: KuduColumnSchema::DataTypeToString(type))
Maybe you can declare this up front so you needn't duplicate it in multiple
case statements.
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@767
PS9, Line 767: KuduColumnSchema col_schema = schema.Column(0);
Could you add a comment here explaining that we're not actually interested in
the first column of the schema, but that there's no default constructor for
KuduColumnSchema so we're forced to create one using some valid column?
http://gerrit.cloudera.org:8080/#/c/13976/9/src/kudu/tools/tool_action_table.cc@797
PS9, Line 797: auto it = kCompressionTypeMap.find(compression_type_uc);
: if (it == kCompressionTypeMap.end()) {
Should be able to use one of the FindOr* functions from gutil/map-util.h here
(and in ColumnSetEncoding).
--
To view, visit http://gerrit.cloudera.org:8080/13976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Thu, 15 Aug 2019 16:50:56 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#9). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: 1. kudu table column_set_default should be provided as a JSON array, e.g. [1] or ["foo"]. 2. kudu table column_remove_default 3. kudu table column_set_compression 4. kudu table column_set_encoding 5. kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 531 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/9 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 9 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#8). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: 1. kudu table column_set_default should be provided as a JSON array, e.g. [1] or ["foo"]. 2. kudu table column_remove_default 3. kudu table column_set_compression 4. kudu table column_set_encoding 5. kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 531 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/8 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 8 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 7:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h
File src/kudu/client/schema.h:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h@583
PS6, Line 583: bool HasColumn(const std::string& col_name, KuduColumnSchema*
col_schema) const;
> Maybe rename to "HasColumn"?
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h@583
PS6, Line 583: e, KuduColumnSchem
> Nit: "KuduColumnSchema* "
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@18
PS6, Line 18: #include
> Nit: prefer cstdlib over this.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@86
PS6, Line 86: "Also check for the existence of the row on the
leader replica of "
: "the tablet. If found, the full row will be
> Good question. We have implemented "sub-modes" (i.e. mode within another mo
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@676
PS6, Line 676: Status DropRangePartition(const RunnerContext& context) {
> Don't need std:: prefix for std::string.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@677
PS6, Line 677: return ModifyRangePartition(context, PartitionAction::DROP);
> Could this be defined statically and using an initializer_list:
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@695
PS6, Line 695: "Only one default value should be provided, but we got $0
value",
: std::to_string(values.size(;
> Nit: indentation
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@698
PS6, Line 698: switch (type) {
> But for STRING/BINARY columns, isn't the empty string a valid value?
If users want to set default value of a STRING/BINARY column to an empty string
, they could remove_default for the column.
I finally set the default_value argument as a JSON array on account of another
problem. That is if we want to set_default for a INT column to a negative
number, such as '-1', the command line tool will parse it as a flag.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@706
PS6, Line 706: reader.ExtractInt64(values[0], /*field=*/nullptr,
_value),
: Substitute("unable to parse value for column type $0",
: KuduColumnSchema::DataTypeToString(type)));
: *value = KuduValue::FromInt(int_value);
: b
> Use safe_strto64() here instead. And use the other safe_ functions below in
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@716
PS6, Line 716: reader.ExtractString(values[0], /*field=*/nullptr,
_value),
> Isn't this guaranteed by L697?
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@720
PS6, Line 720: break;
: }
: case KuduColumnSchema::DataType::BOOL: {
: bool bool_value;
: RETURN_NOT_OK_PREPEND(
> Use boost::iequals() for the comparison to make it case-insensitive.
Here I parse it as a JSON value.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@736
PS6, Line 736:KuduColumnSchema::DataTypeToString(type)));
> Shouldn't this be DataType::DECIMAL? Same above for UNIXTIME_MICROS.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@737
PS6, Line 737: *value = KuduValue::FromFloat(double_value);
: break;
: }
: case KuduColumnSchema::DataType::DOUBLE: {
: double double_value;
: RETURN_NOT_OK_PREPEND(
: reader.ExtractDouble(values[0], /*field=*/n
> These can be combined.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@750
PS6, Line 750: default:
> Same comment about static definition and an initializer_list.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@759
PS6, Line 759: Status ColumnSetDefault(const RunnerContext& context) {
: const string& table_name = FindOrDie(context.required_args,
kTableNameArg);
> This is duplicated.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@779
PS6, Line 779:
> Same.
Done
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@1021
PS6,
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#7). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column for a table by command line tools. The 'delete_column' tool can be used as: kudu table delete_column The alter column tools can be used as: 1. kudu table column_set_default should be provided as a JSON array, e.g. [1] or ["foo"]. 2. kudu table column_remove_default 3. kudu table column_set_compression 4. kudu table column_set_encoding 5. kudu table column_set_block_size Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 532 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/7 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 7 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@86 PS6, Line 86: DEFINE_string(alter_column_args, "", : "Args of alter operation on the column."); > I think it's a good idea. Good question. We have implemented "sub-modes" (i.e. mode within another mode) in a few places, but there has been some pushback from folks who think that it makes it more difficult to discover functionality. Furthermore, rename_column was added in 1.8.0 so moving it into a sub-mode would represent a compatibility break in the CLI. Probably not a major one, but an inconvenient one nonetheless. So I think I'd vote for keeping all of the column actions rooted in the 'table' mode. But I'm open to changing it if others feel strongly about adding a 'column' sub-mode. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 6 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 13 Aug 2019 18:54:23 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@86 PS6, Line 86: DEFINE_string(alter_column_args, "", : "Args of alter operation on the column."); > This isn't very intuitive, which makes me think that the various "alter col I think it's a good idea. What about add a new "column" mode? By doing so, existing "rename_column" tool could also be integrated. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 6 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 13 Aug 2019 11:34:03 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 6:
(18 comments)
I didn't review the test changes.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h
File src/kudu/client/schema.h:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h@583
PS6, Line 583: bool Column(const std::string& col_name, KuduColumnSchema
*col_schema) const;
Maybe rename to "HasColumn"?
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/client/schema.h@583
PS6, Line 583: KuduColumnSchema *
Nit: "KuduColumnSchema* "
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@18
PS6, Line 18: #include
Nit: prefer cstdlib over this.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@86
PS6, Line 86: DEFINE_string(alter_column_args, "",
: "Args of alter operation on the column.");
This isn't very intuitive, which makes me think that the various "alter column"
operations should be implemented as individual tools. It's more code to set up
the various Actions, but it'll allow you to customize the help properly and
better validate the input. Plus with some decomposition there shouldn't be any
duplicated code in the core logic.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@676
PS6, Line 676: Status ParseAlterType(const std::string& str_alter_type,
AlterType* alter_type) {
Don't need std:: prefix for std::string.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@677
PS6, Line 677: // Build a alter_type_map from string to AlterType.
Could this be defined statically and using an initializer_list:
static unordered_map kAlterTypeMap = ({{ "...",
SET_DEFAULT } { ..., ... } });
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@695
PS6, Line 695: KuduColumnSchema::DataType type,
: KuduValue** value) {
Nit: indentation
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@698
PS6, Line 698: return Status::InvalidArgument("The set_default value cannot
be empty.");
But for STRING/BINARY columns, isn't the empty string a valid value?
Would it be helpful if the default value argument was JSON-encoded? Then it'd
be easier to express empty strings.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@706
PS6, Line 706: int64_t int64_value = atoi64(str_value);
: if (std::to_string(int64_value) != str_value) {
: return Status::InvalidArgument(Substitute(
: "Failed to parse value from $0 to a int value.",
str_value));
: }
Use safe_strto64() here instead. And use the other safe_ functions below in the
FLOAT and DOUBLE conversions.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@716
PS6, Line 716: if (!str_value.empty()) {
Isn't this guaranteed by L697?
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@720
PS6, Line 720: case KuduColumnSchema::DataType::BOOL:
: if (str_value == "true") {
: *value = KuduValue::FromBool(true);
: } else if (str_value == "false") {
: *value = KuduValue::FromBool(false);
Use boost::iequals() for the comparison to make it case-insensitive.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@736
PS6, Line 736: case KuduColumnSchema::DECIMAL:
Shouldn't this be DataType::DECIMAL? Same above for UNIXTIME_MICROS.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@737
PS6, Line 737: return Status::NotSupported(
: Substitute("$0 columns are not supported for setting
default value by this tool",
:
KuduColumnSchema::DataTypeToString(type)));
: default:
: return Status::NotSupported(Substitute(
: "Unsupported column type $0 for setting default value.",
: KuduColumnSchema::DataTypeToString(type)));
These can be combined.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@750
PS6, Line 750: // Build a encoding_type_map from string to
KuduColumnStorageAttributes::EncodingType.
Same comment about static definition and an initializer_list.
http://gerrit.cloudera.org:8080/#/c/13976/6/src/kudu/tools/tool_action_table.cc@759
PS6, Line 759: encoding_type_map.emplace(
:
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@710
PS4, Line 710: }
> Ok. Can you add a comment mentioning that decimal is left out intentionally
Done
http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc@86
PS5, Line 86: alter_colu
> nit: so it's extra clear this is referring to columns without reading the d
Done
http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc@839
PS5, Line 839: }
: case AlterType::SET_ENCODING: {
> Can you add negative tests for these, testing that if a user inputs a bad v
Done
http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc@1008
PS5, Line 1008: "If
> nit: fix indentation here
Done
--
To view, visit http://gerrit.cloudera.org:8080/13976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Mon, 12 Aug 2019 10:53:22 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#6). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column write_default value, encoding/compression type and block_size by command line tools. The command can be used as: 1. kudu table delete_column 2. kudu table alter_column [-alter_args=] could be 'set_default', 'remove_default', 'set_compression', 'set_encoding' or 'set_block_size'. Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 510 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/6 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 6 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@710 PS4, Line 710: *value = KuduValue::CopyString(str_value); > I agreed that using Status as return value is more friendly to users, and I Ok. Can you add a comment mentioning that decimal is left out intentionally? Or better yet, have a case for DECIMAL that returns a Status::NotSupported() so users know that decimal isn't supported by this tool yet http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc@86 PS5, Line 86: alter_args nit: so it's extra clear this is referring to columns without reading the description, how about calling this 'alter_column_args'? http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc@839 PS5, Line 839: return Status::InvalidArgument(Substitute( : "Invalid block size: $0, it should be set higher than 0.", FLAGS_alter_args)); Can you add negative tests for these, testing that if a user inputs a bad value, the tool will exit with a reasonable error? Same with the other input parsing (e.g. passing in bad values for a given type, passing in bad values for --alter_args). http://gerrit.cloudera.org:8080/#/c/13976/5/src/kudu/tools/tool_action_table.cc@1008 PS5, Line 1008: unique_ptr alter_column = nit: fix indentation here -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 5 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 08 Aug 2019 19:21:47 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 5: Verified+1 The test failure seems unrelated. -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 5 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 08 Aug 2019 19:10:02 + Gerrit-HasComments: No
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has removed a vote on this change. Change subject: [tools] Add table tools to delete column and alter column .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 5 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/client/schema.cc
File src/kudu/client/schema.cc:
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/client/schema.cc@720
PS4, Line 720:
: bool KuduSchema::Column(const std::string& col_name,
KuduColumnSchema* col_schema) const {
: int idx = schema_->find_column(col_name);
: if (idx == Schema::kColumnNotFound) {
: return false;
:
> Hitting a CHECK or DCHECK as a user is pretty unpleasant. It's a better use
Done
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@183
PS4, Line 183: enum PartitionAction {
: ADD,
> nit: not from this patch, but could you reduce the indent here by two space
Done
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@710
PS4, Line 710: *value = KuduValue::CopyString(str_value);
> Why aren't other types (e.g. bool, binary, decimal) represented here?
I agreed that using Status as return value is more friendly to users, and I
removed all CHECKs and DCHECKs in this patch.
I added bool, binary and unixtime_micros types, the decimal type column is not
supported now for it's quiet difficult to parse input value.
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@732
PS4, Line 732: *value = KuduValue::FromDouble(strtod(str_value.c_str(),
nullptr));
: }
: break;
: default:
: return Status::NotSupported(Substitute(
: "Unsupported column type $0 for setting default value.",
type));
: }
: return Status::OK();
: }
:
: Status ParseSetEncodingArgs(const std::string& args,
:
KuduColumnStorageAttributes::EncodingType* encod
> What do you think about storing these in a map of { string => KuduColumnSt
Done
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@810
PS4, Line 810:
> Should we check that the input here is valid?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Wed, 07 Aug 2019 11:39:46 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#5). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column write_default value, encoding/compression type and block_size by command line tools. The command can be used as: 1. kudu table delete_column 2. kudu table alter_column [-alter_args=] could be 'set_default', 'remove_default', 'set_compression', 'set_encoding' or 'set_block_size'. Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 356 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/5 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 5 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [tools] Add table tools to delete column and alter column
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13976 )
Change subject: [tools] Add table tools to delete column and alter column
..
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/client/schema.cc
File src/kudu/client/schema.cc:
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/client/schema.cc@720
PS4, Line 720:
: KuduColumnSchema KuduSchema::Column(const std::string& col_name)
const {
: int idx = schema_->find_column(col_name);
: DCHECK_NE(idx, Schema::kColumnNotFound);
: return Column(idx);
: }
Hitting a CHECK or DCHECK as a user is pretty unpleasant. It's a better user
experience if the tool were to catch errors and return a message about why it
failed. Maybe consider having this be:
bool KuduSchema::Column(const std::string& col_name, KuduColumnSchema* col) {
int idx = schema_->find_column(col_name);
if (idx == kColumnNotFound) {
return false;
}
*col = Column(idx);
return true;
}
and catching errors when calling this function.
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@183
PS4, Line 183: ADD,
: DROP,
nit: not from this patch, but could you reduce the indent here by two spaces?
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@710
PS4, Line 710: CHECK(false) << Substitute("Unhandled type $0", type);
Why aren't other types (e.g. bool, binary, decimal) represented here?
Also, could you have this return a Status instead of hitting a check failure?
We try to use CHECKs to indicate a programmer error, rather than a user error.
We could also then return Status::InvalidArgument() instead of nullptr in cases
where the values aren't correct. I would recommend combining this function with
ParseSetDefaultArgs() as:
Status ParseArgOfType(const std::string& arg, KuduColumnSchema::DataType type,
KuduValue* value) {
// Switch on the type
// If the value doesn't match the type, return Status::InvalidArgument()
...
return Status::OK();
}
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@732
PS4, Line 732: if (encoding_type_uc == "AUTO_ENCODING") {
: *encoding_type =
KuduColumnStorageAttributes::EncodingType::AUTO_ENCODING;
: } else if (encoding_type_uc == "PLAIN_ENCODING") {
: *encoding_type =
KuduColumnStorageAttributes::EncodingType::PLAIN_ENCODING;
: } else if (encoding_type_uc == "PREFIX_ENCODING") {
: *encoding_type =
KuduColumnStorageAttributes::EncodingType::PREFIX_ENCODING;
: } else if (encoding_type_uc == "RLE") {
: *encoding_type =
KuduColumnStorageAttributes::EncodingType::RLE;
: } else if (encoding_type_uc == "DICT_ENCODING") {
: *encoding_type =
KuduColumnStorageAttributes::EncodingType::DICT_ENCODING;
: } else if (encoding_type_uc == "BIT_SHUFFLE") {
: *encoding_type =
KuduColumnStorageAttributes::EncodingType::BIT_SHUFFLE;
What do you think about storing these in a map of { string =>
KuduColumnStorageAttributes::EncodingType } and iterating through that map?
We could also reuse that map to print out a useful error message, like "Failed
to parse encoding type from , supported encoding types: ".
http://gerrit.cloudera.org:8080/#/c/13976/4/src/kudu/tools/tool_action_table.cc@810
PS4, Line 810: atoi32(FLAGS_alter_args)
Should we check that the input here is valid?
--
To view, visit http://gerrit.cloudera.org:8080/13976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Fri, 02 Aug 2019 03:04:16 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13976 ) Change subject: [tools] Add table tools to delete column and alter column .. Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/13976/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13976/3//COMMIT_MSG@13 PS3, Line 13: kudu table a > kudu table alter_column Done http://gerrit.cloudera.org:8080/#/c/13976/3//COMMIT_MSG@14 PS3, Line 14: alter_type > alter_type Done http://gerrit.cloudera.org:8080/#/c/13976/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13976/3/src/kudu/tools/kudu-tool-test.cc@2927 PS3, Line 2927: dele > alter Done http://gerrit.cloudera.org:8080/#/c/13976/3/src/kudu/tools/kudu-tool-test.cc@3283 PS3, Line 3283: string master_addr = cluster_->master()->bound_rpc_addr().ToString(); > We will not check the writen data, so it's not needed to 'Start' Done http://gerrit.cloudera.org:8080/#/c/13976/3/src/kudu/tools/kudu-tool-test.cc@3364 PS3, Line 3364: ASSERT_STR_NOT_CONTAINS(table->schema().ToString(), kColumnName); > We have to check schema contains this column before deleting it. Done http://gerrit.cloudera.org:8080/#/c/13976/3/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13976/3/src/kudu/tools/tool_action_table.cc@174 PS3, Line 174: NOT_SUPPORTED = 0, > https://google.github.io/styleguide/cppguide.html#Enumerator_Names Done -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 4 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 01 Aug 2019 11:00:01 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table tools to delete column and alter column
Hello Tidy Bot, Yingchun Lai, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13976 to look at the new patch set (#4). Change subject: [tools] Add table tools to delete column and alter column .. [tools] Add table tools to delete column and alter column This patch supports to delete column and alter column write_default value, encoding/compression type and block_size by command line tools. The command can be used as: 1. kudu table delete_column 2. kudu table alter_column [-alter_args=] could be 'set_default', 'remove_default', 'set_compression', 'set_encoding' or 'set_block_size'. Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f --- M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 317 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/13976/4 -- To view, visit http://gerrit.cloudera.org:8080/13976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228340e46fe48ffc782c4c7346f890444b8c550f Gerrit-Change-Number: 13976 Gerrit-PatchSet: 4 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
