Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9240 )

Change subject: Add back KuduColumnSchema DataTypeToString
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9240/2/src/kudu/client/schema.cc
File src/kudu/client/schema.cc:

http://gerrit.cloudera.org:8080/#/c/9240/2/src/kudu/client/schema.cc@568
PS2, Line 568:   return DataType_Name(ToInternalDataType(type, 
type_attributes));
> Isn't this going to return 'Decimal32'/'Decimal64'/'Decimal128'?  I don't t
Yeah, I was mainly trying to maintain existing functionality because I didn't 
fully understand the existing implementation. Before decimal, I am not sure why 
the old implementation called ToInternalDataType on the public data type and 
used DataType_Name. My best guess now that I look deeper is that this was the 
"easiest" way to get a string from the public data type since protobuf 
generated a function for it. That seams a bit contrived though.

Instead I can change DataTypeToString to simply output a string for each public 
DataType enum entry. Is there a more clever method than an array of strings in 
c++?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bf6879f5ab623366a962c6a3d51efc49978a53c
Gerrit-Change-Number: 9240
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Thu, 08 Feb 2018 02:45:15 +0000
Gerrit-HasComments: Yes

Reply via email to