Zoltan Ivanfi has posted comments on this change. Change subject: IMPALA-3973: add position and occurrence to instr() ......................................................................
Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/4094/6/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: PS6, Line 322: search_start_pos > Ok, but since it requires a bit of thinking can you add a DCHECK for #2 to Done http://gerrit.cloudera.org:8080/#/c/4094/10/be/src/runtime/string-search.h File be/src/runtime/string-search.h: PS10, Line 23: #include <cstring> : #include <vector> > can you put the std includes above boost includes, w/ a space btwn? Done PS10, Line 104: for (int i = 0; i <= mlast; ++i) { : BloomAdd(pattern_->ptr[i]); : if (pattern_->ptr[i] == pattern_->ptr[mlast] && i != mlast) skip_ = mlast - i - 1; : if (pattern_->ptr[i] == pattern_->ptr[0] && i != 0) { : rskip_ = std::min(rskip_, i - 1); : } > Given that this is subtle code (at least the algorithm is tricky), I think Done -- 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: 10 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: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Zoltan Ivanfi <[email protected]> Gerrit-HasComments: Yes
