You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2016/11/08 21:41:43 UTC

[kudu-CR] KUDU-1563. Add support for INSERT IGNORE

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1563. Add support for INSERT IGNORE
......................................................................


Patch Set 11:

(3 comments)

only nits and one possible patch split.
Could you address the tidy bot nits?

Thanks for being persistent Brock, almost there :)

http://gerrit.cloudera.org:8080/#/c/4491/11/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 280:     for (uint32_t feature : op->write_op->required_server_features()) {
hummm this smells like it should go on its own patch along with the other feature flag stuff and with a small directed test. Would that be hard?


http://gerrit.cloudera.org:8080/#/c/4491/11/src/kudu/tablet/tablet_random_access-test.cc
File src/kudu/tablet/tablet_random_access-test.cc:

PS11, Line 113: int r = rand() % 3;
move 'r' and its assignment to before the outer if


PS11, Line 114: if (r == 1) {
use switch/case


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bfc35e9d27bd5e2d3375b68e6e4716ed671f36c
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Brock Noland <br...@phdata.io>
Gerrit-Reviewer: Brock Noland <br...@phdata.io>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes