----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4670/#review7373 -----------------------------------------------------------
This is looking pretty close. A few minor comments. http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LOCube.java <https://reviews.apache.org/r/4670/#comment16271> 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. http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanBuilder.java <https://reviews.apache.org/r/4670/#comment16272> you can merge this try/catch with the one above. http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java <https://reviews.apache.org/r/4670/#comment16274> 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..) http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java <https://reviews.apache.org/r/4670/#comment16273> ExecMode.LOCAL iirc http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java <https://reviews.apache.org/r/4670/#comment16275> return here http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java <https://reviews.apache.org/r/4670/#comment16276> Assert.fail("Expected to throw an exception when duplicate dimensions are detected!"); (otherwise your test passes even if this doesn't work!) - Dmitriy On 2012-04-12 07:12:48, Prasanth_J wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4670/ > ----------------------------------------------------------- > > (Updated 2012-04-12 07:12:48) > > > 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 https://issues.apache.org/jira/browse/PIG-2167. > > https://issues.apache.org/jira/browse/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 > >
