Mailing-List: contact dev-help@pig.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@pig.apache.org
Content-Type: multipart/alternative;
 boundary="===============8914473375417424462=="
MIME-Version: 1.0
Subject: Re: Review Request: disable optimizations via pig properties
From: "Travis Crawford" <traviscrawford@gmail.com>
To: "Julien Le Dem" <julien@ledem.net>, "Feng Peng" <pengfeng@gmail.com>,
 "Bill Graham" <billgraham@gmail.com>
Cc: "pig" <dev@pig.apache.org>, "Travis Crawford" <traviscrawford@gmail.com>
Date: Mon, 13 May 2013 23:47:43 -0000
Message-ID: <20130513234743.22989.78598@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Travis Crawford" <noreply@reviews.apache.org>
References: <20130513232329.23050.56583@reviews.apache.org>
In-Reply-To: <20130513232329.23050.56583@reviews.apache.org>
Reply-To: "Travis Crawford" <traviscrawford@gmail.com>

--===============8914473375417424462==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


> On May 13, 2013, 11:23 p.m., Julien Le Dem wrote:
> > src/org/apache/pig/PigConstants.java, lines 47-51
> > <https://reviews.apache.org/r/11032/diff/2/?file=3D290927#file290927lin=
e47>
> >
> >     I would like to clearly separate keys for pig.properties from other=
s in there.
> >     =

> >     Can we either move this one somewhere else, or somehow organize key=
s separately. This one is not used in pig.properties (as you mentioned in t=
he doc).

By move somewhere else, do you mean clearly comment in the code these are i=
nternal constants, or move to a new org.apache.pig.impl.PigInternalConstant=
s class?


> On May 13, 2013, 11:23 p.m., Julien Le Dem wrote:
> > src/org/apache/pig/newplan/logical/optimizer/LogicalPlanOptimizer.java,=
 line 215
> > <https://reviews.apache.org/r/11032/diff/2/?file=3D290929#file290929lin=
e215>
> >
> >     would checkNotNull(ruleSet, "ruleSet") give a better error message ?

I'm generally against checkNotNull in constructors because an NPE suggests =
the method did something wrong. checkArgument signals the caller did someth=
ing wrong. Since this is validating input I'd like to continue using the la=
tter.


> On May 13, 2013, 11:23 p.m., Julien Le Dem wrote:
> > src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java=
, line 263
> > <https://reviews.apache.org/r/11032/diff/2/?file=3D290928#file290928lin=
e263>
> >
> >     This one would be the value for they key.
> >     pigOptimizerRulesDisabledValues ?

Renamed pigOptimizerRulesDisabled.


- Travis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11032/#review20514
-----------------------------------------------------------


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 ca=
n be set in pig.properties, "set" commands in scripts themselves, and comma=
nd-line -D options.
> =

> The use-case is, for scripts that require certain optimizations to be dis=
abled, allowing the script itself to disable the optimization. Currently wh=
atever 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=3D/etc/pig/conf ./bin/pig -c query.pig
> =

> THIS WORKS:
> PIG_CONF_DIR=3D/etc/pig/conf ./bin/pig -Dpig.optimizer.rules.disabled=3DC=
olumnMapKeyPrune -c query.pig
> =

> Notice how "-Dpig.optimizer.rules.disabled=3DColumnMapKeyPrune" 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(GruntParse=
r.java:419)
> 	at org.apache.pig.tools.grunt.GruntParser.processExplain(GruntParser.jav=
a: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(DelegatingMethodAcces=
sorImpl.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.compil=
e(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$Projection=
Rewriter.visit(ProjectionPatcher.java:91)
> 	at org.apache.pig.newplan.logical.expression.ProjectExpression.accept(Pr=
ojectExpression.java:207)
> 	at org.apache.pig.newplan.DepthFirstWalker.depthFirst(DepthFirstWalker.j=
ava: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(A=
llExpressionVisitor.java:142)
> 	at org.apache.pig.newplan.logical.relational.LOInnerLoad.accept(LOInnerL=
oad.java:128)
> 	at org.apache.pig.newplan.DependencyOrderWalker.walk(DependencyOrderWalk=
er.java:75)
> 	at org.apache.pig.newplan.logical.optimizer.AllExpressionVisitor.visit(A=
llExpressionVisitor.java:124)
> 	at org.apache.pig.newplan.logical.relational.LOForEach.accept(LOForEach.=
java:76)
> 	at org.apache.pig.newplan.DependencyOrderWalker.walk(DependencyOrderWalk=
er.java:75)
> 	at org.apache.pig.newplan.PlanVisitor.visit(PlanVisitor.java:52)
> 	at org.apache.pig.newplan.logical.optimizer.ProjectionPatcher.transforme=
d(ProjectionPatcher.java:48)
> 	at org.apache.pig.newplan.optimizer.PlanOptimizer.optimize(PlanOptimizer=
.java:113)
> 	... 13 more
> =

> =

> Thanks,
> =

> Travis Crawford
> =

>


--===============8914473375417424462==--