Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14306 )

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


Patch Set 21:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/14306/21/src/kudu/tools/tool_action_table.cc@937
PS21, Line 937:         return Status::InvalidArgument(Substitute(
> I'm sorry, I made a mistake. If the integer value of enum is invalid, the o
Here are some experiments I ran with code I shared earlier:

  $ ./test '{c : 1}'
  Attempting to parse JSON: '{c : 1}'
  c: RED
  $ ./test '{c : 2}'
  Attempting to parse JSON: '{c : 2}'
  c: GREEN
  $ ./test '{c : 3}'
  Attempting to parse JSON: '{c : 3}'
  unable to parse JSON: c: invalid value  3 for type TYPE_ENUM

This was with protoc 3.0.0; when I switched to protoc 3.4.0 (from Kudu's tree) 
I got the "pass through" behavior you're talking about:

  $ ./test '{c : 1}'
  Attempting to parse JSON: '{c : 1}'
  c: RED
  $ ./test '{c : 2}'
  Attempting to parse JSON: '{c : 2}'
  c: GREEN
  $ ./test '{c : 3}'
  Attempting to parse JSON: '{c : 3}'
  1: 3

Anyway, we need to handle this. 
https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#enum
 has some useful advice: call bool Foo_IsValid(int value) on the enum after 
parsing it from JSON. If we just parsed a string value, it'll always be valid. 
But if we parsed an int value, we might now have an invalid enum that we should 
reject.



--
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: 21
Gerrit-Owner: YangSong <sy1...@yeah.net>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy1...@yeah.net>
Gerrit-Comment-Date: Fri, 18 Oct 2019 04:38:35 +0000
Gerrit-HasComments: Yes

Reply via email to