saketa.chalamch...@gmail.com 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. I'll make a new change request -- 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: saketa.chalamch...@gmail.com Gerrit-Reviewer: Chris George <chris.geo...@rms.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: saketa.chalamch...@gmail.com Gerrit-Comment-Date: Fri, 17 Nov 2017 23:21:54 +0000 Gerrit-HasComments: No
