Alexey Serbin has posted comments on this change.

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

Patch Set 9:

File src/kudu/common/

Line 47:   typedef Status (KuduPartialRow::*BinarySetter)(int, const Slice&);
> OK I understand why this is needed now, based on our Slack conversation. So
Yep, that's the fun part about std::function which I didn't know before I tried 
to introduce the suggested std::function change.

Line 47:   typedef Status (KuduPartialRow::*BinarySetter)(int, const Slice&);
> hrm, would you mind removing the space between Status and the opening paren
The Status is separated by a space because it's a return type.  I don't think 
removing the space after that would bring in any clarity.

These typedefs are for static_casts<> which you can find below in the code.  
It's not possible to instantiate std::function wrapper for overloaded function 
without an explicit cast.  The cast is necessary to allow the compiler to deal 
with ambiguity in the presence of overloaded functions and instantiate the 
appropriate template when compiling the code with std::functor wrapper for the 
overloaded functions.

Line 78:     case COPY:
> style nit: indent case per

Line 98:     case COPY:
> same

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Dinesh Bhat <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to