> On Aug. 16, 2012, 10:40 a.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReduceOper.java, > > line 144 > > <https://reviews.apache.org/r/5523/diff/10/?file=139850#file139850line144> > > > > Same here, a more semantically oriented comment would be better. > > Something like: > > Indicates that there is a rank operation in the MR job.
Actually it is never used. I delete it. > On Aug. 16, 2012, 10:40 a.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/PORank.java, > > line 25 > > <https://reviews.apache.org/r/5523/diff/10/?file=139857#file139857line25> > > > > I would add here the fact that this PO relies on being run in a > > specific MR class because it accesses the counters. Actually POCounter relies on a specific MR class > On Aug. 16, 2012, 10:40 a.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LORank.java, > > line 130 > > <https://reviews.apache.org/r/5523/diff/10/?file=139863#file139863line130> > > > > What do you mean by 'Legal'? changed > On Aug. 16, 2012, 10:40 a.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LORank.java, > > line 175 > > <https://reviews.apache.org/r/5523/diff/10/?file=139863#file139863line175> > > > > Now that I think of it, we should have some validity checks here. > > The operator can't be at the same time a dense rank and a row number. > > I would put the other flag off in the set method, i.e.: > > in setIsDenseRank() you also set this.isRowNumber = false; and log a > > warning saying that something is strange because this should not happen. I will validate it on LogicalPlanGenerator where these values are set. - aavendan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5523/#review10398 ----------------------------------------------------------- On Aug. 14, 2012, 8:19 a.m., aavendan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5523/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2012, 8:19 a.m.) > > > Review request for pig, aavendan and Gianmarco De Francisci Morales. > > > Description > ------- > > Review board for https://issues.apache.org/jira/browse/PIG-2353 > > > This addresses bug PIG-2353. > https://issues.apache.org/jira/browse/PIG-2353 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MRCompiler.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReduceOper.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PhyPlanSetter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapReduceCounter.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/DotPOPrinter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/PhyPlanVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/PlanPrinter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POCounter.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/PORank.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/AllExpressionVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/AllSameRalationalNodesVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/LogicalPlanPrinter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/SchemaResetter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/UidResetter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LORank.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LogToPhyTranslationVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalRelationalNodesVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/rules/ColumnPruneHelper.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/rules/ColumnPruneVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/LineageFindRelVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/ProjectStarExpander.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/SchemaAliasVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/TypeCheckingRelVisitor.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstPrinter.g > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstValidator.g > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanBuilder.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryLexer.g > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/pen/IllustratorAttacher.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/pen/LocalMapReduceSimulator.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/tools/pigstats/ScriptState.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/e2e/pig/deployers/ExistingClusterDeployer.pm > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/e2e/pig/deployers/LocalDeployer.pm > 1372471 > http://svn.apache.org/repos/asf/pig/trunk/test/e2e/pig/tests/nightly.conf > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/e2e/pig/tools/generate/generate_data.pl > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLexer.pig > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLogicalPlanGenerator.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestParser.pig > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryLexer.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryParser.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/OptimizeLimitPlanPrinter.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestLogicalPlanBuilder.java > 1372471 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestOrderBy3.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestRank1.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestRank2.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestRank3.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/5523/diff/ > > > Testing > ------- > > All pre-commit tests passed > > > Thanks, > > aavendan > >