Lars Volker has posted comments on this change. Change subject: IMPALA-3973: add position and occurrence to instr() ......................................................................
Patch Set 9: (1 comment) I'm good with fixing the style in a subsequent change. > > I would suggest clang-formatting this file in a follow-up change. > This way we separate the formatting changes cleanly in the sense > that every part of this diff clearly means to a functional change, > while every part of the follow-up change will purely reflect a > formatting change. > > > I'm not sure whether you should > > adapt all of string-search.h to our style guide, now that a > > significant part of the file changed. I'd be in favor of it, but > I > > don't feel strongly about it. http://gerrit.cloudera.org:8080/#/c/4094/9/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: PS9, Line 329: IntVal StringFunctions::Instr(FunctionContext* context, const StringVal& str, : const StringVal& substr, const BigIntVal& start_position) { : return Instr(context, str, substr, start_position, BigIntVal(1)); : } : : IntVal StringFunctions::Instr( : FunctionContext* context, const StringVal& str, const StringVal& substr) { : return Instr(context, str, substr, BigIntVal(1), BigIntVal(1)); : } > Hmmm, this was wrapped by clang-format, I have no idea why the wrapping is Yes, I agree. Let's keep it as it is. -- To view, visit http://gerrit.cloudera.org:8080/4094 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9648de458d243306fa14adc5e7f7002bf6f67fd Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zoltan Ivanfi <[email protected]> Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Zoltan Ivanfi <[email protected]> Gerrit-HasComments: Yes
