Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
......................................................................
Patch Set 2:
(3 comments)
Can we add basic test coverage for SetString() and SetBinary()?
http://gerrit.cloudera.org:8080/#/c/3868/2/src/kudu/client/stubs.h
File src/kudu/client/stubs.h:
Line 66: // For deprecated functions or variables, generate a warning at usage
sites.
If you want to bring in this helper as part of this patch, just mention it in
the commit message.
Also, I wonder why this is defined both in port.h and here... maybe we should
not define it here if it's not used in the client.
http://gerrit.cloudera.org:8080/#/c/3868/2/src/kudu/common/partial_row.cc
File src/kudu/common/partial_row.cc:
Line 277: return SetSliceCopy<TypeTraits<BINARY> >(col_name, val);
Can we just delegate these calls to SetBinaryCopy() so it's clear it's just an
alias? Or vice versa?
http://gerrit.cloudera.org:8080/#/c/3868/2/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
Line 92: // Sets the binary/binary value, copying 'val' data immediately.
Essentially,
typo: binary/binary
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-HasComments: Yes
