Todd Lipcon has posted comments on this change. Change subject: IMPALA-4134,IMPALA-3704: Kudu INSERT improvements ......................................................................
Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4728/4/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: Line 132: KUDU_RETURN_IF_ERROR(s, "Unable to set flush mode"); // Return error status for RELEASE > We have the same error weirdness for the other session_->Set*() calls. I'm fwiw, our reasoning for making even simple setter calls return Status is for API compatibility. Imagine that you want to write an app which runs against multiple versions of the Kudu client API. Old versions don't support this flush mode. Without a returned Status, we'd end up having to crash or something. Here we can return a reasonable status saying that the flush mode isn't supported. For the case of Impala it's not as big a deal since you bundle a specific client version, but the Kudu API is publicly consumable and a lot of people would be linking against /usr/lib/libkudu_client.so rather than bundling. -- To view, visit http://gerrit.cloudera.org:8080/4728 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5542b9a061b01c543a139e8722560b1365f06595 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
