You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Yuxiang Wang <yw...@nyu.edu> on 2017/04/06 21:05:39 UTC
Re: Review Request 58199: PIG-5214: search any substring in the input
substring
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58199/
-----------------------------------------------------------
(Updated \u56db\u6708 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
Re: Review Request 58199: PIG-5214: search any substring in the input
substring
Posted by Yuxiang Wang <yw...@nyu.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58199/
-----------------------------------------------------------
(Updated \u56db\u6708 12, 2017, 5:23 p.m.)
Review request for pig and Yuxiang Wang.
Repository: pig-git
Description
-------
See PIG-5214.
Diffs (updated)
-----
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/3/
Changes: https://reviews.apache.org/r/58199/diff/2-3/
Testing
-------
Thanks,
Yuxiang Wang
Re: Review Request 58199: PIG-5214: search any substring in the input
substring
Posted by Yuxiang Wang <yw...@nyu.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58199/
-----------------------------------------------------------
(Updated \u56db\u6708 7, 2017, 10:33 p.m.)
Review request for pig and Yuxiang Wang.
Changes
-------
Updated Diff!
Repository: pig-git
Description
-------
See PIG-5214.
Diffs (updated)
-----
CHANGES.txt 3f3521662
src/docs/src/documentation/content/xdocs/basic.xml 0264089a3
src/docs/src/documentation/content/xdocs/pig-index.xml 8e831e257
Diff: https://reviews.apache.org/r/58199/diff/2/
Changes: https://reviews.apache.org/r/58199/diff/1-2/
Testing
-------
Thanks,
Yuxiang Wang
Re: Review Request 58199: PIG-5214: search any substring in the input
substring
Posted by Yuxiang Wang <yw...@nyu.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58199/
-----------------------------------------------------------
(Updated \u56db\u6708 7, 2017, 10:23 p.m.)
Review request for pig and Yuxiang Wang.
Changes
-------
Uploaded a new patch file after several failures to UPDATED my current Diff.
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
-------
File Attachments (updated)
----------------
PIG-5214-2.patch
https://reviews.apache.org/media/uploaded/files/2017/04/07/f345b747-7ae8-4ce1-b68a-20b0db3f2723__PIG-5214-2.patch
Thanks,
Yuxiang Wang
Re: Review Request 58199: PIG-5214: search any substring in the input
substring
Posted by Daniel Dai <da...@gmail.com>.
-----------------------------------------------------------
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
>
>