Alexey Serbin has uploaded a new change for review.

Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data

[KuduPartialRow::Set{Binary,String}()] copy input data

KuduPartialRow::SetBinary()/SetString() behavior was optimized
to omit copying of the passed data.  However, a user of the API
might assume these methods are safe to use along with other setters
as SetInt32, SetDouble, etc. where the string or binary data
goes out of scope (or deallocated) before AppendToPB() is called.
To play safe, the behavior of these methods has been changed
to immediately copy the input data.

This is safe, but an incompatible change, but we don't bump
API/ABI version since Kudu is not 1.0 yet.

This approach may add some performance regression issues for
already existing clients, but hopefully it is negligible for most
of the C++ clients around.  As for the Impala-Kudu integration,
it seems current Impala code uses SetString() only in one place
in be/src/sec/, and that can be addressed

An alternative approach might be to deprecate
KuduPartialRow::Set{Binary,String}() in favor of the newly introduced
KuduPartialRow::Set{Binary,String}NoCopy() methods.  That approach
would spare us from performance regression worries and compatibility
issues.  However, after some discussion, introducing the copying
behavior of KuduPartialRow::SetBinary()/SetString() methods
appeared to be more beneficial in the long run
from the usability perspective.

Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
M src/kudu/client/
M src/kudu/client/stubs.h
M src/kudu/common/
M src/kudu/common/
M src/kudu/common/
M src/kudu/common/partial_row.h
M src/kudu/common/
M src/kudu/gutil/port.h
M src/kudu/integration-tests/
M src/kudu/integration-tests/
M src/kudu/integration-tests/
M src/kudu/master/
M src/kudu/tablet/
M src/kudu/tserver/
14 files changed, 108 insertions(+), 61 deletions(-)

  git pull ssh:// refs/changes/68/3868/1
To view, visit
To unsubscribe, visit

Gerrit-MessageType: newchange
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <>

Reply via email to