YangSong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14273 )

Change subject: add a tool to create table
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14273/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/14273/5/src/kudu/tools/tool_action_table.cc@897
PS5, Line 897:   RETURN_NOT_OK(reader.ExtractString(reader.root(), "TableName", 
&table_name));
> Yes, the idea is that rather than doing this piece-by-piece parsing with Js
I know what you mean, thank you. But I have a problem, JsonStringToMessage can 
convert one type JSON into one type PB object, here column type is variable, 
default value of column and range split value depend on it, maybe I can define 
them to string type in the JSON, but the range split also has InBound and 
ExBound two types. Now Range Split can be defined as:
"RangeSplit": [
        {
            "InLowerBound": "1",
            "ExUpperBound": "2"
        },
        {
            "InLowerBound": "2",
            "InUpperBound": "3"
        }
]
but if using PB, it maybe change as:

"RangeSplit": [
        {
            "LowerBound": {
                  "BoundType":"Include",
                  "BoundVaule":"1"
             },
            "UpperBound": {
                  "BoundType":"Exclude",
                  "BoundVaule":"2"
             }
        },
        {
            "LowerBound": {
                  "BoundType":"Include",
                  "BoundVaule":"2"
             },
            "UpperBound": {
                  "BoundType":"Include",
                  "BoundVaule":"3"
             }
        }
]
So I find the PB object can't be used directly and it become even more 
complicated.


http://gerrit.cloudera.org:8080/#/c/14273/5/src/kudu/tools/tool_action_table.cc@1392
PS5, Line 1392:       .AddRequiredParameter({ kCreateFileNameArg, "Name of the 
table file" })
> I agree that it's easy to make a mistake when parsing the JSON, but I just
OK, I will try to support both, default reading from a JSON object.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacd9ae64dc07e3da6efe840e4bbcfaf3df0ecdc7
Gerrit-Change-Number: 14273
Gerrit-PatchSet: 5
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: Mon, 23 Sep 2019 11:24:42 +0000
Gerrit-HasComments: Yes

Reply via email to