Todd Lipcon has posted comments on this change.

Change subject: Changes: In kudu-spark, added support to delete rows from Kudu 
table  in DataSource API. This is done using the SaveMode.ErrorIfExists.
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3552/2//COMMIT_MSG
Commit Message:

Line 7: Changes: In kudu-spark, added support to delete rows from Kudu table 
nit: please follow the standard commit message format (one-line short summary, 
followed by a blank line, followed by the list of changes). See 
http://chris.beams.io/posts/git-commit/


Line 11: Usage :
this usage info is probably better to put in the doc for the data frame, or 
perhaos in docs/developing.adoc


Line 14:   .mode(SaveMode.ErrorIfExists).kudu
Why are you using 'ErrorIfExists' here? It's not what is documented for that 
save mode ("ErrorIfExists mode means that when saving a DataFrame to a data 
source, if data already exists, an exception is expected to be thrown.")


Line 21:     * If there are additional columns along with primary columns, 
seems a little unintuitive here to ignore the extra columns, IMO. what do other 
people think?


http://gerrit.cloudera.org:8080/#/c/3552/2/java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala:

Line 229:         for ((sparkIdx, kuduIdx) <- indices) {
this stuff is all duplicated - can you refactor it into a utility method?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf7207d5ee525c07b54f7544e0ba63deb47e6acd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu <ram.me...@rms.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to