You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Jonathan Coveney <jc...@gmail.com> on 2013/02/18 18:50:10 UTC
Review Request: Introduce a syntax making declared aliases optional
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9496/
-----------------------------------------------------------
Review request for pig, Daniel Dai and Alan Gates.
Description
-------
https://issues.apache.org/jira/browse/PIG-3136
This addresses bug PIG-3136.
https://issues.apache.org/jira/browse/PIG-3136
Diffs
-----
src/org/apache/pig/PigServer.java 847a126
src/org/apache/pig/parser/LogicalPlanBuilder.java b705686
src/org/apache/pig/parser/LogicalPlanGenerator.g 01dc570
src/org/apache/pig/parser/QueryLexer.g bdd0836
src/org/apache/pig/parser/QueryParser.g ba2fec9
src/org/apache/pig/parser/QueryParserDriver.java a250b73
src/org/apache/pig/tools/grunt/GruntParser.java 2049a46
src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj 109a3b2
test/org/apache/pig/test/TestBuiltin.java 9c5d408
Diff: https://reviews.apache.org/r/9496/diff/
Testing
-------
ant -Dtestcase=TestBuiltin test, and ant test-commit
I made them TestBuiltin use it
Thanks,
Jonathan Coveney
Re: Review Request: Introduce a syntax making declared aliases optional
Posted by Jonathan Coveney <jc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9496/
-----------------------------------------------------------
(Updated March 4, 2013, 2:55 p.m.)
Review request for pig, Daniel Dai and Alan Gates.
Description
-------
https://issues.apache.org/jira/browse/PIG-3136
This addresses bug PIG-3136.
https://issues.apache.org/jira/browse/PIG-3136
Diffs (updated)
-----
src/org/apache/pig/PigServer.java a46cc2a
src/org/apache/pig/parser/LogicalPlanBuilder.java b705686
src/org/apache/pig/parser/LogicalPlanGenerator.g 01dc570
src/org/apache/pig/parser/QueryLexer.g bdd0836
src/org/apache/pig/parser/QueryParser.g ba2fec9
src/org/apache/pig/parser/QueryParserDriver.java a250b73
src/org/apache/pig/tools/grunt/GruntParser.java 2658466
src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj 109a3b2
test/org/apache/pig/test/TestBuiltin.java 9c5d408
test/org/apache/pig/test/TestShortcuts.java 7d66ef7
Diff: https://reviews.apache.org/r/9496/diff/
Testing
-------
ant -Dtestcase=TestBuiltin test, and ant test-commit
I made them TestBuiltin use it
Thanks,
Jonathan Coveney
Re: Review Request: Introduce a syntax making declared aliases optional
Posted by Jonathan Coveney <jc...@gmail.com>.
> On March 1, 2013, 11:13 p.m., Cheolsoo Park wrote:
> > Hi Jonathan,
> >
> > Overall looks good. My only concern is lack of unit tests. Indeed, you converted some test cases in TestBuiltin, but that doesn't seem to cover dump, describe, explain, illustrate. I am wondering whether you can add basic test cases somewhere. PIG-2994 added TestShortcuts, and that might be a good place to add new test cases?
> >
> > Let me know what you think.
Totally agree. Will do.
> On March 1, 2013, 11:13 p.m., Cheolsoo Park wrote:
> > src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj, line 250
> > <https://reviews.apache.org/r/9496/diff/2/?file=263924#file263924line250>
> >
> > Please update (" ")* with (" " | "\t")*. This is fine in interactive mode since Grunt shell doesn't allow tab chars. However, it will throw an error when executing a Pig script that contains tab chars in batch mode. For example, if I have a Pig script called test.pig:
> >
> > -- tab after fat arrow
> > => load '1.txt';
> > dump @;
> >
> > ./bin/pig -x local test.pig
> >
> > This fails because of the tab char is not recognized.
> >
> > But the following script works:
> >
> > -- tab after equal
> > a = load '1.txt';
> > dump a;
Good point, will do. I'm much more cargo culting in javacc than antlr :)
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9496/#review17244
-----------------------------------------------------------
On March 1, 2013, 10:32 a.m., Jonathan Coveney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9496/
> -----------------------------------------------------------
>
> (Updated March 1, 2013, 10:32 a.m.)
>
>
> Review request for pig, Daniel Dai and Alan Gates.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/PIG-3136
>
>
> This addresses bug PIG-3136.
> https://issues.apache.org/jira/browse/PIG-3136
>
>
> Diffs
> -----
>
> src/org/apache/pig/PigServer.java a46cc2a
> src/org/apache/pig/parser/LogicalPlanBuilder.java b705686
> src/org/apache/pig/parser/LogicalPlanGenerator.g 01dc570
> src/org/apache/pig/parser/QueryLexer.g bdd0836
> src/org/apache/pig/parser/QueryParser.g ba2fec9
> src/org/apache/pig/parser/QueryParserDriver.java a250b73
> src/org/apache/pig/tools/grunt/GruntParser.java 2658466
> src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj 109a3b2
> test/org/apache/pig/test/TestBuiltin.java 9c5d408
>
> Diff: https://reviews.apache.org/r/9496/diff/
>
>
> Testing
> -------
>
> ant -Dtestcase=TestBuiltin test, and ant test-commit
>
> I made them TestBuiltin use it
>
>
> Thanks,
>
> Jonathan Coveney
>
>
Re: Review Request: Introduce a syntax making declared aliases optional
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9496/#review17244
-----------------------------------------------------------
Hi Jonathan,
Overall looks good. My only concern is lack of unit tests. Indeed, you converted some test cases in TestBuiltin, but that doesn't seem to cover dump, describe, explain, illustrate. I am wondering whether you can add basic test cases somewhere. PIG-2994 added TestShortcuts, and that might be a good place to add new test cases?
Let me know what you think.
src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj
<https://reviews.apache.org/r/9496/#comment36655>
Please update (" ")* with (" " | "\t")*. This is fine in interactive mode since Grunt shell doesn't allow tab chars. However, it will throw an error when executing a Pig script that contains tab chars in batch mode. For example, if I have a Pig script called test.pig:
-- tab after fat arrow
=> load '1.txt';
dump @;
./bin/pig -x local test.pig
This fails because of the tab char is not recognized.
But the following script works:
-- tab after equal
a = load '1.txt';
dump a;
- Cheolsoo Park
On March 1, 2013, 10:32 a.m., Jonathan Coveney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9496/
> -----------------------------------------------------------
>
> (Updated March 1, 2013, 10:32 a.m.)
>
>
> Review request for pig, Daniel Dai and Alan Gates.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/PIG-3136
>
>
> This addresses bug PIG-3136.
> https://issues.apache.org/jira/browse/PIG-3136
>
>
> Diffs
> -----
>
> src/org/apache/pig/PigServer.java a46cc2a
> src/org/apache/pig/parser/LogicalPlanBuilder.java b705686
> src/org/apache/pig/parser/LogicalPlanGenerator.g 01dc570
> src/org/apache/pig/parser/QueryLexer.g bdd0836
> src/org/apache/pig/parser/QueryParser.g ba2fec9
> src/org/apache/pig/parser/QueryParserDriver.java a250b73
> src/org/apache/pig/tools/grunt/GruntParser.java 2658466
> src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj 109a3b2
> test/org/apache/pig/test/TestBuiltin.java 9c5d408
>
> Diff: https://reviews.apache.org/r/9496/diff/
>
>
> Testing
> -------
>
> ant -Dtestcase=TestBuiltin test, and ant test-commit
>
> I made them TestBuiltin use it
>
>
> Thanks,
>
> Jonathan Coveney
>
>
Re: Review Request: Introduce a syntax making declared aliases optional
Posted by Jonathan Coveney <jc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9496/
-----------------------------------------------------------
(Updated March 1, 2013, 10:32 a.m.)
Review request for pig, Daniel Dai and Alan Gates.
Description
-------
https://issues.apache.org/jira/browse/PIG-3136
This addresses bug PIG-3136.
https://issues.apache.org/jira/browse/PIG-3136
Diffs (updated)
-----
src/org/apache/pig/PigServer.java a46cc2a
src/org/apache/pig/parser/LogicalPlanBuilder.java b705686
src/org/apache/pig/parser/LogicalPlanGenerator.g 01dc570
src/org/apache/pig/parser/QueryLexer.g bdd0836
src/org/apache/pig/parser/QueryParser.g ba2fec9
src/org/apache/pig/parser/QueryParserDriver.java a250b73
src/org/apache/pig/tools/grunt/GruntParser.java 2658466
src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj 109a3b2
test/org/apache/pig/test/TestBuiltin.java 9c5d408
Diff: https://reviews.apache.org/r/9496/diff/
Testing
-------
ant -Dtestcase=TestBuiltin test, and ant test-commit
I made them TestBuiltin use it
Thanks,
Jonathan Coveney