Ram Mettu has posted comments on this change.

Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col 
For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects 
to receive Array[Byte], so change is to return a copy of the byte array. 
Modified testcase to add missing col ty
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4145/1//COMMIT_MSG
Commit Message:

PS1, Line 7: Fix DataFrame read failure when table has Binary Col
> Would you mind including one or two sentences explaining what the cause was
Done


http://gerrit.cloudera.org:8080/#/c/4145/1/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:

PS1, Line 58: <dependency>
            :             <groupId>org.apache.kudu</groupId>
            :             <artifactId>kudu-client</artifactId>
            :             <version>${project.version}</version>
            :         </dependency>
> I don't think we need this. See comment in KuduContextTest.scala.
Done


http://gerrit.cloudera.org:8080/#/c/4145/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
File 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala:

PS1, Line 38:     assert(r.apply(0).asInstanceO
> I think I'd rather have the Test basic kuduRDD read back all the columns, n
Done


PS1, Line 43: 5).asInstanceOf[Boo
> I don't think we need a full HiveContext. See L57 of DefaultSourceTest.scal
Done


PS1, Line 47: es)
> While we don't have style guidelines for Scala, the pattern has been to use
Done


http://gerrit.cloudera.org:8080/#/c/4145/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
File 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala:

PS1, Line 110: row.addShort(6, i.toShort)
> Could you vary this like like we do for the string type? Odd/even variation
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu <ram.me...@rms.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu <ram.me...@rms.com>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to