YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14306 )
Change subject: add a tool to create table ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool.proto File src/kudu/tools/tool.proto: http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool.proto@346 PS3, Line 346: // lower bound value in format of string, include the value. message RangeBoundPB { message BoundPB { enum Type { INCLUSIVE = 0; EXCLUSIVE = 1; } optional Type bound_type = 1; optional string bound_value = 2; } optional BoundPB lower_bound = 1; optional BoundPB upper_bound = 2; } Thanks for helping. I modify here to support multi-column in range key. But is using enum unfriendly here? http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool.proto@378 PS3, Line 378: optional After removing "require" and "optional", there are errors reported at compile time. So I change the "require" to "optional". I'm not sure if this is the right change. http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool_action_table.cc@196 PS3, Line 196: I define two variables from local to global. I'm not sure if it's right. http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool_action_table.cc@882 PS3, Line 882: if (json_arg[0] == '$') { Here I misunderstood your suggestion. Maybe I just support the JSON object, the JSON file can use 'cat' convert to JSON object. I will modify later. http://gerrit.cloudera.org:8080/#/c/14306/3/src/kudu/tools/tool_action_table.cc@978 PS3, Line 978: // TODO: Mod ConvertToKuduPartialRow's parameter as > warning: missing username/bug in TODO [google-readability-todo] if using ConvertToKuduPartialRow, parameters need to be modified. I list an example. Also first arg(const vector<pair<string, DataType>>& columns ) is OK. -- To view, visit http://gerrit.cloudera.org:8080/14306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0bce4733a504f8ef5f024089a16fe3c6f1e493f1 Gerrit-Change-Number: 14306 Gerrit-PatchSet: 3 Gerrit-Owner: YangSong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: YangSong <[email protected]> Gerrit-Comment-Date: Fri, 27 Sep 2019 07:55:17 +0000 Gerrit-HasComments: Yes
