YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14306 )
Change subject: add a tool to create table ...................................................................... Patch Set 6: (2 comments) I try to use ValuePB to modify the code, the code now can work, but it looks unfriendly to user. So not all the code is done, like support decimal type and the test code. And some of the modified code is retained. http://gerrit.cloudera.org:8080/#/c/14306/6/src/kudu/client/schema.cc File src/kudu/client/schema.cc: http://gerrit.cloudera.org:8080/#/c/14306/6/src/kudu/client/schema.cc@665 PS6, Line 665: LOG(FATAL) << "Unhandled type " << type; I think this place needs to define a invalid data type in the enum, such as "UNKNOWN_DATA = 999" in the common.proto. We need return the invalid data type when user use a wrong parameter. The encode type and compression type have the same problem. http://gerrit.cloudera.org:8080/#/c/14306/6/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/14306/6/src/kudu/tools/kudu-tool-test.cc@5433 PS6, Line 5433: "default_value": {"int_v": 1} Here I define a new PB: message ValuePB { oneof value { int64 int_v = 1; double double_v = 3; bool bool_v = 4; string string_v = 5; } } but in the JSON, the value must be defined as `"default_value": {"int_v": 1}`. That's also user-unfriendly. If using by a PB Any type, it also need be defined as `"default_value": {1}`, it also doesn't look good. Maybe I need use the string type instead of oneof type. -- 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: 6 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: Tue, 08 Oct 2019 09:49:59 +0000 Gerrit-HasComments: Yes
