> On June 28, 2012, 2:07 p.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java, > > line 146 > > <https://reviews.apache.org/r/5523/diff/3/?file=116086#file116086line146> > > > > The same constant is defined in PigMapOnly.MapRank > > It should be defined only in one place.
I will put it at JobControlCompiler and call its value on MapRank > On June 28, 2012, 2:07 p.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java, > > line 342 > > <https://reviews.apache.org/r/5523/diff/3/?file=116086#file116086line342> > > > > What if we have more than one job doing a rank in the same MROoPlan at > > the same time? > > I think this piece of code would not work as you would overwrite the > > rankGroup. I renamed the group of counters with the jobID > On June 28, 2012, 2:07 p.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java, > > line 719 > > <https://reviews.apache.org/r/5523/diff/3/?file=116086#file116086line719> > > > > Logging all this information at INFO level is too much. > > I would either reduce the amount of logged info or put it at DEBUG > > level. deleted > On June 28, 2012, 2:07 p.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PhyPlanSetter.java, > > line 127 > > <https://reviews.apache.org/r/5523/diff/3/?file=116089#file116089line127> > > > > Why is this commented? > > What would be the purpose of this piece of code? I have to check this visitor, the impact on the code. > On June 28, 2012, 2:07 p.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POCounter.java, > > line 44 > > <https://reviews.apache.org/r/5523/diff/3/?file=116094#file116094line44> > > > > Is POCounter responsible for sorting the input? > > it was an initial approach, now I'm considering what is necessary for the POCounter > On June 28, 2012, 2:07 p.m., Gianmarco De Francisci Morales wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POCounter.java, > > line 120 > > <https://reviews.apache.org/r/5523/diff/3/?file=116094#file116094line120> > > > > What role do the rankPlans play in POCounter? It is unclear to me. It's a mistake naming the variables. Cleaning code! - aavendan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5523/#review8710 ----------------------------------------------------------- On June 25, 2012, 10:46 a.m., aavendan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5523/ > ----------------------------------------------------------- > > (Updated June 25, 2012, 10:46 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 > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MRCompiler.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReduceOper.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PhyPlanSetter.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapOnly.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/DotPOPrinter.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/PhyPlanVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/PlanPrinter.java > 1353202 > > 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 > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/AllSameRalationalNodesVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/LogicalPlanPrinter.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/SchemaResetter.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/optimizer/UidResetter.java > 1353202 > > 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 > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalRelationalNodesVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/rules/ColumnPruneHelper.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/rules/ColumnPruneVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/LineageFindRelVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/ProjectStarExpander.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/SchemaAliasVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/TypeCheckingRelVisitor.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstPrinter.g > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstValidator.g > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanBuilder.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryLexer.g > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/tools/pigstats/ScriptState.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLexer.pig > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLogicalPlanGenerator.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestParser.pig > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryLexer.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryParser.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/OptimizeLimitPlanPrinter.java > 1353202 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestLogicalPlanBuilder.java > 1353202 > > 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 > > Diff: https://reviews.apache.org/r/5523/diff/ > > > Testing > ------- > > All pre-commit tests passed > > > Thanks, > > aavendan > >
