Re: Review Request: PIG-2167 - Naive implementation of CUBE operator

[j....@gmail.com]

> On 2012-04-30 00:54:08, Dmitriy Ryaboy wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LOCube.java, line 74
> > <https://reviews.apache.org/r/4670/diff/1-2/?file=100633#file100633line74>
> >
> >     I see, you cut this out in LogicalPlanGenerator so you assume this won't get called before the plan is generated. That'll probably work for now. We should definitely fix this in the next stage.

Yeah. Will fix that in the next stage (implementing physical operator).


> On 2012-04-30 00:54:08, Dmitriy Ryaboy wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanBuilder.java, line 561
> > <https://reviews.apache.org/r/4670/diff/1-2/?file=100639#file100639line561>
> >
> >     you can merge this try/catch with the one above.

Done.


> On 2012-04-30 00:54:08, Dmitriy Ryaboy wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java, line 58
> > <https://reviews.apache.org/r/4670/diff/1-2/?file=100648#file100648line58>
> >
> >     try Util.createFile(String[] data)
> >     
> >     Even better, you can now use the new MockLoader from PIG-2650 (save a bit more time and complexity on the tests..)

Updated. Now all test cases use MockLoader.


> On 2012-04-30 00:54:08, Dmitriy Ryaboy wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java, line 302
> > <https://reviews.apache.org/r/4670/diff/1-2/?file=100648#file100648line302>
> >
> >     return here

Updated.


> On 2012-04-30 00:54:08, Dmitriy Ryaboy wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java, line 318
> > <https://reviews.apache.org/r/4670/diff/1-2/?file=100648#file100648line318>
> >
> >     Assert.fail("Expected to throw an exception when duplicate dimensions are detected!");
> >     
> >     (otherwise your test passes even if this doesn't work!)

Updated. 


> On 2012-04-30 00:54:08, Dmitriy Ryaboy wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java, line 74
> > <https://reviews.apache.org/r/4670/diff/1-2/?file=100648#file100648line74>
> >
> >     ExecMode.LOCAL iirc

PigServer constructor accepts string ('local') or ExecType (ExecType.LOCAL). If its a string it internally converts to corresponding ExecType. 


- Prasanth_J


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4670/#review7373
-----------------------------------------------------------


On 2012-04-30 04:05:03, Prasanth_J wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4670/
> -----------------------------------------------------------
> 
> (Updated 2012-04-30 04:05:03)
> 
> 
> Review request for pig and Dmitriy Ryaboy.
> 
> 
> Summary
> -------
> 
> This is a review board for https://issues.apache.org/jira/browse/PIG-2167
> 
> 
> This addresses bug PIG-2167.
>     https://issues.apache.org/jira/browse/PIG-2167
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/AllExpressionVisitor.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/SchemaResetter.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LOCube.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalRelationalNodesVisitor.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/ProjectStarExpander.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstPrinter.g 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstValidator.g 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanBuilder.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryLexer.g 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLexer.pig 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLogicalPlanGenerator.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestParser.pig 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryLexer.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryParser.java 1325115 
>   http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/4670/diff
> 
> 
> Testing
> -------
> 
> Unit tests: All passed
> 
> Pre-commit tests: All passed
> ant clean test-commit
> 
> 
> Thanks,
> 
> Prasanth_J
> 
>