----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58199/#review171313 -----------------------------------------------------------
build.xml Line 315 (original), 315 (patched) <https://reviews.apache.org/r/58199/#comment244206> Changes in build.xml is not related to the patch, please revert it. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 28 (patched) <https://reviews.apache.org/r/58199/#comment244217> We also need to add doc to src/docs/src/documentation/content/xdocs/func.xml. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 29 (patched) <https://reviews.apache.org/r/58199/#comment244216> We need javadoc for builtin udfs. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 30 (patched) <https://reviews.apache.org/r/58199/#comment244207> It might be better to name it REGEX_SEARCH src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 33 (patched) <https://reviews.apache.org/r/58199/#comment244208> I cannot imagine what is the use case of mUserMatches=true in case of find. In find, the patten will definitely not match the whole string. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 54 (patched) <https://reviews.apache.org/r/58199/#comment244210> I think we shall allow pattern to include parentheses. In case parentheses are missing, you may quote them in parentheses. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 61 (patched) <https://reviews.apache.org/r/58199/#comment244209> It seems there is tab character in your code. Pig only use space for indent. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 87 (patched) <https://reviews.apache.org/r/58199/#comment244211> We can do a little better here, inner schema for the bag is known, you can use the following code to specify output schema: org.apache.pig.impl.util.Utils.getSchemaFromString("{(match:chararray)}") src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 89 (patched) <https://reviews.apache.org/r/58199/#comment244212> We shall not silently return null in case of exception. You can throw RuntimeException is needed. src/org/apache/pig/builtin/STRING_SEARCH_ALL.java Lines 98 (patched) <https://reviews.apache.org/r/58199/#comment244213> Remove this unrelated line. test/org/apache/pig/test/TestBuiltin.java Lines 1998 (patched) <https://reviews.apache.org/r/58199/#comment244214> Shall align with the next line. test/org/apache/pig/test/TestBuiltin.java Lines 2000 (patched) <https://reviews.apache.org/r/58199/#comment244215> bagFactory is never used, remove - Daniel Dai On April 6, 2017, 9:05 p.m., Yuxiang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58199/ > ----------------------------------------------------------- > > (Updated April 6, 2017, 9:05 p.m.) > > > Review request for pig and Yuxiang Wang. > > > Repository: pig-git > > > Description > ------- > > See PIG-5214. > > > Diffs > ----- > > build.xml e70aa9979 > src/org/apache/pig/builtin/STRING_SEARCH_ALL.java PRE-CREATION > test/org/apache/pig/test/TestBuiltin.java fbc3f1e03 > > > Diff: https://reviews.apache.org/r/58199/diff/1/ > > > Testing > ------- > > > Thanks, > > Yuxiang Wang > >
