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

Change subject: KUDU-1737 : Submit column characteristics via KuduContext
......................................................................


Patch Set 3:

> java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
 >
 > - Reflowed all lines to be < 100 characters
 > - Removed comments
 > - Introduced constants for property names
 > - "Line 125:
 > how does this 'AnyRef' thing work? are you sure this works with all
 > types? Can you try something like an int8 column and passing an
 > integer?"
 >
 > AnyRef doesn't work with BinaryType and returns and incorrectly
 > converts integer to int8 in the scenario mentioned above.
 > Therefore, removed "AnyRef" and cast DefaultValue to it's right
 > type using Column Type as reference. Appropriate errors are thrown
 > in case of type mismatch.
 >
 > - Encoding and Compression Algorithm values are converted to Upper
 > case to accomodate for both upper and lower characters in input
 >
 > Removed
 > java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextWithMetadataTest.scala
 >
 > Added test cases under 
 > java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
 >
 > - Reflowed all lines to be < 100 characters
 > - Added test cases for valid and invalid metadata columns
 > - Followed variable names used in other test cases
 > - "Is there a nicer way to specify the metadata than using
 > fromJson? I'm surprised you can't pass it as a scala map."
 >
 > There is. Kind of. The MetaDataBuilder can build metadata with the
 > following put methods
 > putBoolean
 > putBooleanArray
 > putDouble
 > putDoubleArray
 > putLong
 > putLongArray
 > putMetadata
 > putMetadataArray
 > putString
 > putStringArray

Does it make sense to start a new code review? since the base is out of date 
and is hard to tell what are your changes on the diff.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddbf60d61ac3f7e14b91fd2f83137c2dc8f1ecd5
Gerrit-Change-Number: 6010
Gerrit-PatchSet: 3
Gerrit-Owner: [email protected]
Gerrit-Reviewer: Chris George <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Comment-Date: Fri, 17 Nov 2017 23:18:31 +0000
Gerrit-HasComments: No

Reply via email to