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

Reply via email to