You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Richard Ding <rd...@yahoo-inc.com> on 2011/02/07 19:13:40 UTC
Review Request: Add macro expansion to Pig Latin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/400/
-----------------------------------------------------------
Review request for pig and Julien Le Dem.
Summary
-------
As production Pig scripts grow longer and longer, Pig Latin has a need to integrate standard programming techniques of separation and code sharing offered by functions and modules. A detailed proposal of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
This addresses bug PIG-1793.
https://issues.apache.org/jira/browse/PIG-1793
Diffs
-----
http://svn.apache.org/repos/asf/pig/trunk/build.xml 1067233
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java 1067233
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1067233
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java 1067233
http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java PRE-CREATION
Diff: https://reviews.apache.org/r/400/diff
Testing
-------
Thanks,
Richard
Re: Review Request: Add macro expansion to Pig Latin
Posted by Julien Le Dem <ju...@ledem.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/400/#review211
-----------------------------------------------------------
Ship it!
- Julien
On 2011-02-08 11:26:41, Richard Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/400/
> -----------------------------------------------------------
>
> (Updated 2011-02-08 11:26:41)
>
>
> Review request for pig and Julien Le Dem.
>
>
> Summary
> -------
>
> As production Pig scripts grow longer and longer, Pig Latin has a need to integrate standard programming techniques of separation and code sharing offered by functions and modules. A detailed proposal of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
>
>
> This addresses bug PIG-1793.
> https://issues.apache.org/jira/browse/PIG-1793
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/pig/trunk/build.xml 1068533
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java 1068533
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1068533
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java 1068533
> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/400/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Richard
>
>
Re: Review Request: Add macro expansion to Pig Latin
Posted by Richard Ding <rd...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/400/
-----------------------------------------------------------
(Updated 2011-02-08 11:26:41.125217)
Review request for pig and Julien Le Dem.
Changes
-------
Attaching new patch that addresses the review comments.
Summary
-------
As production Pig scripts grow longer and longer, Pig Latin has a need to integrate standard programming techniques of separation and code sharing offered by functions and modules. A detailed proposal of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
This addresses bug PIG-1793.
https://issues.apache.org/jira/browse/PIG-1793
Diffs (updated)
-----
http://svn.apache.org/repos/asf/pig/trunk/build.xml 1068533
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java 1068533
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java PRE-CREATION
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1068533
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java 1068533
http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java PRE-CREATION
Diff: https://reviews.apache.org/r/400/diff
Testing
-------
Thanks,
Richard
Re: Review Request: Add macro expansion to Pig Latin
Posted by Alan Gates <ga...@yahoo-inc.com>.
On Feb 7, 2011, at 5:05 PM, Richard Ding wrote:
>
>
>> On 2011-02-07 15:25:52, Julien Le Dem wrote:
>>> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
>>> , line 3
>>> <https://reviews.apache.org/r/400/diff/1/?file=10791#file10791line3>
>>>
>>> Can a Macro call another Macro ?
>>> This would be useful.
>>>
>>>
>
> Macro recursion is not supported.
>
>
Does this mean not now, or not period? The plan was certainly to
support macro recursion in this release.
Alan.
Re: Review Request: Add macro expansion to Pig Latin
Posted by Richard Ding <rd...@yahoo-inc.com>.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java, line 74
> > <https://reviews.apache.org/r/400/diff/1/?file=10791#file10791line74>
> >
> > what if I call gamma macro_group_and_count_D_0 instead ?
> > Would it fail?
Yes. The masked alias names are internal.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java, line 3
> > <https://reviews.apache.org/r/400/diff/1/?file=10791#file10791line3>
> >
> > Can a Macro call another Macro ?
> > This would be useful.
> >
> >
Macro recursion is not supported.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g, line 22
> > <https://reviews.apache.org/r/400/diff/1/?file=10784#file10784line22>
> >
> > Is there a way to avoid duplication here ?
This seems to be a short coming of ANTLR. One has to duplicate code for all tree parsers.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java, line 47
> > <https://reviews.apache.org/r/400/diff/1/?file=10787#file10787line47>
> >
> > What does this do ?
This was used to drive the lexer. It's now changed to an explicit while loop.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java, line 507
> > <https://reviews.apache.org/r/400/diff/1/?file=10783#file10783line507>
> >
> > expandMacros should probably return a Reader.
> > Or this replacement code should be put in a local method to reduce duplication.
Add a new method.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java, line 66
> > <https://reviews.apache.org/r/400/diff/1/?file=10787#file10787line66>
> >
> > What about ".foo/..." ?
> > What about "~" ?
"~" is not supported. The file path should be in one of the three forms: (1) absolute; (2) relative to the current directory (start with . or ..), and (3) can be concatenated with the paths specified in the search path.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java, line 82
> > <https://reviews.apache.org/r/400/diff/1/?file=10788#file10788line82>
> >
> > the error message should contain the actual mismatching values to help with debugging
sure
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java, line 99
> > <https://reviews.apache.org/r/400/diff/1/?file=10788#file10788line99>
> >
> > Catching individual exception may help with better error messages
will do
On 2011-02-07 15:25:52, Richard Ding wrote:
> > Does the following work:
> > - Can a Macro call another Macro ?
> > - Can a Macro take a Macro in parameter?
Answer is NO and NO. We intentionally call it macro instead function to disallow recursion.
- Richard
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/400/#review189
-----------------------------------------------------------
On 2011-02-07 10:13:39, Richard Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/400/
> -----------------------------------------------------------
>
> (Updated 2011-02-07 10:13:39)
>
>
> Review request for pig and Julien Le Dem.
>
>
> Summary
> -------
>
> As production Pig scripts grow longer and longer, Pig Latin has a need to integrate standard programming techniques of separation and code sharing offered by functions and modules. A detailed proposal of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
>
>
> This addresses bug PIG-1793.
> https://issues.apache.org/jira/browse/PIG-1793
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/pig/trunk/build.xml 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java 1067233
> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/400/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Richard
>
>
Re: Review Request: Add macro expansion to Pig Latin
Posted by Alan Gates <ga...@yahoo-inc.com>.
Cool.
Alan.
On Feb 8, 2011, at 9:54 AM, Richard Ding wrote:
>
>
>> On 2011-02-07 15:25:52, Julien Le Dem wrote:
>>> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
>>> , line 3
>>> <https://reviews.apache.org/r/400/diff/1/?file=10791#file10791line3>
>>>
>>> Can a Macro call another Macro ?
>>> This would be useful.
>>>
>>>
>>
>> Richard Ding wrote:
>> Macro recursion is not supported.
>
> Let me clarify: Macro inlining is not a complete replacement for
> function calls, as it does not support recursive calls. However,
> macros will be allowed to inline other macros (but forward
> declarations will not be supported). This feature is not in the
> current patch and will be added in the next patch.
>
>
> - Richard
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/400/#review189
> -----------------------------------------------------------
>
>
> On 2011-02-07 10:13:39, Richard Ding wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/400/
>> -----------------------------------------------------------
>>
>> (Updated 2011-02-07 10:13:39)
>>
>>
>> Review request for pig and Julien Le Dem.
>>
>>
>> Summary
>> -------
>>
>> As production Pig scripts grow longer and longer, Pig Latin has a
>> need to integrate standard programming techniques of separation and
>> code sharing offered by functions and modules. A detailed proposal
>> of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
>>
>>
>> This addresses bug PIG-1793.
>> https://issues.apache.org/jira/browse/PIG-1793
>>
>>
>> Diffs
>> -----
>>
>> http://svn.apache.org/repos/asf/pig/trunk/build.xml 1067233
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java
>> 1067233
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g
>> PRE-CREATION
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g
>> PRE-CREATION
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g
>> PRE-CREATION
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java
>> PRE-CREATION
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java
>> PRE-CREATION
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g
>> 1067233
>> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java
>> 1067233
>> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
>> PRE-CREATION
>>
>> Diff: https://reviews.apache.org/r/400/diff
>>
>>
>> Testing
>> -------
>>
>>
>> Thanks,
>>
>> Richard
>>
>>
>
Re: Review Request: Add macro expansion to Pig Latin
Posted by Richard Ding <rd...@yahoo-inc.com>.
> On 2011-02-07 15:25:52, Julien Le Dem wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java, line 3
> > <https://reviews.apache.org/r/400/diff/1/?file=10791#file10791line3>
> >
> > Can a Macro call another Macro ?
> > This would be useful.
> >
> >
>
> Richard Ding wrote:
> Macro recursion is not supported.
Let me clarify: Macro inlining is not a complete replacement for function calls, as it does not support recursive calls. However, macros will be allowed to inline other macros (but forward declarations will not be supported). This feature is not in the current patch and will be added in the next patch.
- Richard
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/400/#review189
-----------------------------------------------------------
On 2011-02-07 10:13:39, Richard Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/400/
> -----------------------------------------------------------
>
> (Updated 2011-02-07 10:13:39)
>
>
> Review request for pig and Julien Le Dem.
>
>
> Summary
> -------
>
> As production Pig scripts grow longer and longer, Pig Latin has a need to integrate standard programming techniques of separation and code sharing offered by functions and modules. A detailed proposal of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
>
>
> This addresses bug PIG-1793.
> https://issues.apache.org/jira/browse/PIG-1793
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/pig/trunk/build.xml 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java 1067233
> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/400/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Richard
>
>
Re: Review Request: Add macro expansion to Pig Latin
Posted by Julien Le Dem <ju...@ledem.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/400/#review189
-----------------------------------------------------------
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java
<https://reviews.apache.org/r/400/#comment354>
expandMacros should probably return a Reader.
Or this replacement code should be put in a local method to reduce duplication.
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g
<https://reviews.apache.org/r/400/#comment355>
Is there a way to avoid duplication here ?
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java
<https://reviews.apache.org/r/400/#comment356>
What does this do ?
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java
<https://reviews.apache.org/r/400/#comment357>
What about ".foo/..." ?
What about "~" ?
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java
<https://reviews.apache.org/r/400/#comment359>
the error message should contain the actual mismatching values to help with debugging
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java
<https://reviews.apache.org/r/400/#comment360>
Catching individual exception may help with better error messages
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java
<https://reviews.apache.org/r/400/#comment358>
do we need specific exception types here?
http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
<https://reviews.apache.org/r/400/#comment363>
Can a Macro call another Macro ?
This would be useful.
http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
<https://reviews.apache.org/r/400/#comment361>
I like the syntax :)
http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
<https://reviews.apache.org/r/400/#comment362>
what if I call gamma macro_group_and_count_D_0 instead ?
Would it fail?
http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
<https://reviews.apache.org/r/400/#comment365>
pretty cool
Does the following work:
- Can a Macro call another Macro ?
- Can a Macro take a Macro in parameter?
- Julien
On 2011-02-07 10:13:39, Richard Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/400/
> -----------------------------------------------------------
>
> (Updated 2011-02-07 10:13:39)
>
>
> Review request for pig and Julien Le Dem.
>
>
> Summary
> -------
>
> As production Pig scripts grow longer and longer, Pig Latin has a need to integrate standard programming techniques of separation and code sharing offered by functions and modules. A detailed proposal of adding macro expansion to Pig Latin is posted here: http://wiki.apache.org/pig/TuringCompletePig
>
>
> This addresses bug PIG-1793.
> https://issues.apache.org/jira/browse/PIG-1793
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/pig/trunk/build.xml 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java PRE-CREATION
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1067233
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java 1067233
> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/400/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Richard
>
>