Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14273 )
Change subject: add a tool to create table ...................................................................... Patch Set 5: (1 comment) 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)); > I know what you mean, thank you. But I have a problem, JsonStringToMessage OK, I see what you're saying. The problem is that the PB type for a column's default value and for the values associated with range splits/partitions are dependent on other values in the message (i.e. the column types). One ugly work around is to define a generic KuduValue PB message which itself is a oneof on every possible PK type. That doesn't work super well when multiple Kudu types can be represented by a single PB type though (e.g. INT8/INT16/INT32 can all be represented by PB int32). As an alternative, maybe the value is always of type bytes or string (like you suggested) and then we'll need to do some follow-on validation and deserialization after deserializing the main JSON blob. I think even that is still better than the piecemeal approach taken here, because the PB schema is a useful artifact that describes the expected structure of the create table request. -- 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: Tue, 24 Sep 2019 00:42:13 +0000 Gerrit-HasComments: Yes
