> On May 13, 2013, 11:35 p.m., Bill Graham wrote: > > src/docs/src/documentation/content/xdocs/perf.xml, line 493 > > <https://reviews.apache.org/r/11032/diff/2/?file=290925#file290925line493> > > > > Would you please specify that setting this value in both the pig > > properties file and the command line (or script) will be additive. > > Travis Crawford wrote: > Currently it works like this: > > (a) -optimizer_off command-line rules are always disabled. > (b) The "pig.optimizer.rules.disabled" property works like other > properties, where setting in the script itself overwrites previously set > values (from either the command-line or pig.properties). > > Disabled rules are additive in that (a) + (b) will be disabled. However, > within (b) only the last specified value of pig.optimizer.rules.disabled > takes effect. > > I think this makes sense for how people will want to use the feature (and > I think is consistent with how other properties work). > > * Site administrators can specify default rules to disable via > pig.properties. > * Individual scripts can override the site defaults if needed. > * Invokers of pig can supplement the rules to disable. > > Thoughts? If we want to be additive within (b) we'd also need a way to > remove defaults set by site administrators, since the default should be a > suggestion not requirement. That would easily be achieved with a "-" prefix > that would remove disabled rules, but I think we've covered the common > use-cases without introducing extra complexity. > > Bill Graham wrote: > > (b) The "pig.optimizer.rules.disabled" property works like other > properties, where setting in the script itself overwrites previously set > values (from either the command-line or pig.properties). > > This implies SET in a script will override the command line (or > properties). > > > Disabled rules are additive in that (a) + (b) will be disabled. > However, within (b) only the last specified value of > pig.optimizer.rules.disabled takes effect. > > This implies SET in a script (or properties) would be additive with the > command line. > > Can you help clarify what I think sounds like a contradiction? Just > trying to understand the implemented behavior more than propose a change to > it. > > Travis Crawford wrote: > RE: "This implies SET in a script will override the command line (or > properties)." > A: Rules disabled via the -optimizer_off command-line flag are treated > separately from values disabled via the pig property. > > RE: "This implies SET in a script (or properties) would be additive with > the command line." > A: Correct - SET in a script is additive with rules disabled via the > command-line flag. > > > Rules to disable are the set of rules disabled on the command line + > rules disabled via the "pig.optimizer.rules.disabled" property. > > Pig's code currently uses a command-line flag to disable optimization > rules rather than standard pig properties. I think the ideal state would be > using a single property to disable rules because properties are how pig > configuration works in general. However, since there's currently a > command-line flag to disable rules it seems like we should keep it (perhaps > deprecating to allow removing in a future release). > > The proposal in this change is to: > > * preserve existing behavior by making the command-line flag continues to > disable rules as it does today > * Add a new property that also lets you disable optimization rules. This > is a standard pig property that can be set in all the ways one can currently > set properties. > > Then we add rules disabled via the command-line flag with rules disabled > via the property and that's the full list of rules to disable.
Got it, thanks. In your original text I was overlooking the fact that you were discussing a flag which is different then the property. All's clear now. The behavior makes sense to me. We should add a line to the docs to mention that when both -optimizer_off and pig.optimizer.rules.disabled are set, that the union of the two rule sets are disabled. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11032/#review20516 ----------------------------------------------------------- On May 13, 2013, 8:35 p.m., Travis Crawford wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11032/ > ----------------------------------------------------------- > > (Updated May 13, 2013, 8:35 p.m.) > > > Review request for pig, Julien Le Dem, Bill Graham, and Feng Peng. > > > Description > ------- > > Update pig to allow disabling optimizations via pig properties. Currently > optimizations must be disabled via command-line options. Pig properties can > be set in pig.properties, "set" commands in scripts themselves, and > command-line -D options. > > The use-case is, for scripts that require certain optimizations to be > disabled, allowing the script itself to disable the optimization. Currently > whatever runs the script needs to specially handle disabling the optimization > for that specific query. > > > This addresses bug PIG-3317. > https://issues.apache.org/jira/browse/PIG-3317 > > > Diffs > ----- > > src/docs/src/documentation/content/xdocs/perf.xml 108ae7e > src/org/apache/pig/Main.java f97ed9f > src/org/apache/pig/PigConstants.java ea77e97 > src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java > 4dab4e8 > src/org/apache/pig/newplan/logical/optimizer/LogicalPlanOptimizer.java > d26f381 > test/org/apache/pig/test/TestEvalPipeline2.java 39cf807 > > Diff: https://reviews.apache.org/r/11032/diff/ > > > Testing > ------- > > Manually tested on a fully-distributed cluster. > > THIS FAILS: > PIG_CONF_DIR=/etc/pig/conf ./bin/pig -c query.pig > > THIS WORKS: > PIG_CONF_DIR=/etc/pig/conf ./bin/pig > -Dpig.optimizer.rules.disabled=ColumnMapKeyPrune -c query.pig > > Notice how "-Dpig.optimizer.rules.disabled=ColumnMapKeyPrune" specifies a pig > property, which could be in pig.properties, or the script itself. > > > Failure message: > > Pig Stack Trace > --------------- > ERROR 2229: Couldn't find matching uid -1 for project (Name: Project Type: > bytearray Uid: 97550 Input: 0 Column: 1) > > org.apache.pig.impl.logicalLayer.FrontendException: ERROR 1067: Unable to > explain alias null > at org.apache.pig.PigServer.explain(PigServer.java:1057) > at > org.apache.pig.tools.grunt.GruntParser.explainCurrentBatch(GruntParser.java:419) > at > org.apache.pig.tools.grunt.GruntParser.processExplain(GruntParser.java:351) > at org.apache.pig.tools.grunt.Grunt.checkScript(Grunt.java:98) > at org.apache.pig.Main.run(Main.java:607) > at org.apache.pig.Main.main(Main.java:152) > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) > at > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) > at java.lang.reflect.Method.invoke(Method.java:597) > at org.apache.hadoop.util.RunJar.main(RunJar.java:186) > Caused by: org.apache.pig.impl.logicalLayer.FrontendException: ERROR 2000: > Error processing rule ColumnMapKeyPrune. Try -t ColumnMapKeyPrune > at > org.apache.pig.newplan.optimizer.PlanOptimizer.optimize(PlanOptimizer.java:122) > at > org.apache.pig.backend.hadoop.executionengine.HExecutionEngine.compile(HExecutionEngine.java:281) > at org.apache.pig.PigServer.compilePp(PigServer.java:1380) > at org.apache.pig.PigServer.explain(PigServer.java:1042) > ... 10 more > Caused by: org.apache.pig.impl.logicalLayer.FrontendException: ERROR 2229: > Couldn't find matching uid -1 for project (Name: Project Type: bytearray Uid: > 97550 Input: 0 Column: 1) > at > org.apache.pig.newplan.logical.optimizer.ProjectionPatcher$ProjectionRewriter.visit(ProjectionPatcher.java:91) > at > org.apache.pig.newplan.logical.expression.ProjectExpression.accept(ProjectExpression.java:207) > at > org.apache.pig.newplan.DepthFirstWalker.depthFirst(DepthFirstWalker.java:64) > at > org.apache.pig.newplan.DepthFirstWalker.walk(DepthFirstWalker.java:53) > at org.apache.pig.newplan.PlanVisitor.visit(PlanVisitor.java:52) > at > org.apache.pig.newplan.logical.optimizer.AllExpressionVisitor.visit(AllExpressionVisitor.java:142) > at > org.apache.pig.newplan.logical.relational.LOInnerLoad.accept(LOInnerLoad.java:128) > at > org.apache.pig.newplan.DependencyOrderWalker.walk(DependencyOrderWalker.java:75) > at > org.apache.pig.newplan.logical.optimizer.AllExpressionVisitor.visit(AllExpressionVisitor.java:124) > at > org.apache.pig.newplan.logical.relational.LOForEach.accept(LOForEach.java:76) > at > org.apache.pig.newplan.DependencyOrderWalker.walk(DependencyOrderWalker.java:75) > at org.apache.pig.newplan.PlanVisitor.visit(PlanVisitor.java:52) > at > org.apache.pig.newplan.logical.optimizer.ProjectionPatcher.transformed(ProjectionPatcher.java:48) > at > org.apache.pig.newplan.optimizer.PlanOptimizer.optimize(PlanOptimizer.java:113) > ... 13 more > > > Thanks, > > Travis Crawford > >
