You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2017/11/17 23:42:46 UTC

[kudu-CR] Bug: KUDU-1737

saketa.chalamchala@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8591


Change subject: Bug: KUDU-1737
......................................................................

Bug: KUDU-1737

Submit defaultValue, desiredBlockSize, encoding and
compressionAlgorithm via metadata in schema when using KuduContext's
createTable.

Change-Id: Idfca6018e68cf654141e381dde167b0e99b7b07a
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
2 files changed, 316 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/8591/1
-- 
To view, visit http://gerrit.cloudera.org:8080/8591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfca6018e68cf654141e381dde167b0e99b7b07a
Gerrit-Change-Number: 8591
Gerrit-PatchSet: 1
Gerrit-Owner: saketa.chalamchala@gmail.com

[kudu-CR] Bug: KUDU-1737

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
saketa.chalamchala@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/8591 )

Change subject: Bug: KUDU-1737
......................................................................


Patch Set 1:

> It looks like you've split this change across two gerrits. Does
 > this one obsolete the earlier one? If so, can you mark that one as
 > abandoned? Did you address all of the requested changes from the
 > prior one?

The previous one was obsolete. I abandoned it.
I made the requested changes. Please find a summary of the changes below : 

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
- 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 a MetaDataBuilder that can build metadata with the following put methods
putBoolean
putBooleanArray
putDouble
putDoubleArray
putLong
putLongArray
putMetadata
putMetadataArray
putString
putStringArray


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idfca6018e68cf654141e381dde167b0e99b7b07a
Gerrit-Change-Number: 8591
Gerrit-PatchSet: 1
Gerrit-Owner: saketa.chalamchala@gmail.com
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: saketa.chalamchala@gmail.com
Gerrit-Comment-Date: Mon, 20 Nov 2017 23:58:21 +0000
Gerrit-HasComments: No

[kudu-CR] Bug: KUDU-1737

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8591 )

Change subject: Bug: KUDU-1737
......................................................................


Patch Set 1:

It looks like you've split this change across two gerrits. Does this one obsolete the earlier one? If so, can you mark that one as abandoned? Did you address all of the requested changes from the prior one?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idfca6018e68cf654141e381dde167b0e99b7b07a
Gerrit-Change-Number: 8591
Gerrit-PatchSet: 1
Gerrit-Owner: saketa.chalamchala@gmail.com
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 20 Nov 2017 19:35:46 +0000
Gerrit-HasComments: No