> 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 > >
