Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13760 )

Change subject: KUDU-1938 Add support for CHAR/VARCHAR pt 1
......................................................................


Patch Set 23:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13760/23/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:

http://gerrit.cloudera.org:8080/#/c/13760/23/src/kudu/common/partial_row.h@153
PS23, Line 153:   /// @note The copying behavior is new for these methods 
starting Kudu 0.10.
              :   ///   Prior to Kudu 0.10, these methods behaved like
              :   ///   KuduPartialRow::SetStringNoCopy() and 
KuduPartialRow::SetBinaryNoCopy()
              :   ///   correspondingly.
This @note doesn't apply to the new methods you added (below too). Perhaps we 
should split out Char and Varchar from the setters here into a separate section 
to avoid the confusion?

On a related note, do you think we should add NoCopy variants of the new 
functions?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I998982dba93831db91c43a97ce30d3e68c2a4a54
Gerrit-Change-Number: 13760
Gerrit-PatchSet: 23
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Mon, 22 Jul 2019 19:09:37 +0000
Gerrit-HasComments: Yes

Reply via email to