You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pig.apache.org by th...@apache.org on 2011/06/25 02:06:20 UTC
svn commit: r1139469 - in /pig/trunk: ./
src/org/apache/pig/newplan/logical/expression/
src/org/apache/pig/newplan/logical/relational/
src/org/apache/pig/newplan/logical/rules/ test/org/apache/pig/test/
Author: thejas
Date: Sat Jun 25 00:06:20 2011
New Revision: 1139469
URL: http://svn.apache.org/viewvc?rev=1139469&view=rev
Log:
PIG-2139: LogicalExpressionSimplifier optimizer rule should check if udf is
deterministic while checking if they are equal
Modified:
pig/trunk/CHANGES.txt
pig/trunk/src/org/apache/pig/newplan/logical/expression/UserFuncExpression.java
pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalPlan.java
pig/trunk/src/org/apache/pig/newplan/logical/rules/OptimizerUtils.java
pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java
Modified: pig/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/pig/trunk/CHANGES.txt?rev=1139469&r1=1139468&r2=1139469&view=diff
==============================================================================
--- pig/trunk/CHANGES.txt (original)
+++ pig/trunk/CHANGES.txt Sat Jun 25 00:06:20 2011
@@ -60,6 +60,9 @@ PIG-2011: Speed up TestTypedMap.java (dv
BUG FIXES
+PIG-2139: LogicalExpressionSimplifier optimizer rule should check if udf is
+ deterministic while checking if they are equal (thejas)
+
PIG-2137: SAMPLE should not be pushed above DISTINCT (dvryaboy and thejas)
PIG-2136: Implementation of Sample should use LessThanExpression
Modified: pig/trunk/src/org/apache/pig/newplan/logical/expression/UserFuncExpression.java
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/newplan/logical/expression/UserFuncExpression.java?rev=1139469&r1=1139468&r2=1139469&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/newplan/logical/expression/UserFuncExpression.java (original)
+++ pig/trunk/src/org/apache/pig/newplan/logical/expression/UserFuncExpression.java Sat Jun 25 00:06:20 2011
@@ -18,10 +18,12 @@
package org.apache.pig.newplan.logical.expression;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.apache.pig.EvalFunc;
import org.apache.pig.FuncSpec;
+import org.apache.pig.builtin.Nondeterministic;
import org.apache.pig.data.DataType;
import org.apache.pig.impl.PigContext;
import org.apache.pig.impl.logicalLayer.FrontendException;
@@ -67,6 +69,16 @@ public class UserFuncExpression extends
@Override
public boolean isEqual(Operator other) throws FrontendException {
+
+ //For the purpose of optimization rules (specially LogicalExpressionSimplifier)
+ // a non deterministic udf is not equal to another. So returning false for
+ //such cases.
+ // Note that the function is also invoked by implementations of OperatorPlan.isEqual
+ // that function is called from test cases to compare logical plans, and
+ // it will return false even if the plans are clones.
+ if(!this.isDeterministic())
+ return false;
+
if( other instanceof UserFuncExpression ) {
UserFuncExpression exp = (UserFuncExpression)other;
if (!mFuncSpec.equals(exp.mFuncSpec ))
@@ -74,6 +86,14 @@ public class UserFuncExpression extends
List<Operator> mySuccs = getPlan().getSuccessors(this);
List<Operator> theirSuccs = other.getPlan().getSuccessors(other);
+ if(mySuccs == null || theirSuccs == null){
+ if(mySuccs == null && theirSuccs == null){
+ return true;
+ }else{
+ //only one of the udfs has null successors
+ return false;
+ }
+ }
if (mySuccs.size()!=theirSuccs.size())
return false;
for (int i=0;i<mySuccs.size();i++) {
@@ -85,6 +105,22 @@ public class UserFuncExpression extends
return false;
}
}
+
+ public boolean isDeterministic() throws FrontendException{
+ Class<?> udfClass;
+ try {
+ udfClass = PigContext.resolveClassName(getFuncSpec().getClassName());
+ }catch(IOException ioe) {
+ throw new FrontendException("Cannot instantiate: " + getFuncSpec(), ioe) ;
+ }
+
+ if (udfClass.getAnnotation(Nondeterministic.class) == null) {
+ return true;
+ }
+ return false;
+
+ }
+
public List<LogicalExpression> getArguments() throws FrontendException {
List<Operator> successors = null;
Modified: pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalPlan.java
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalPlan.java?rev=1139469&r1=1139468&r2=1139469&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalPlan.java (original)
+++ pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalPlan.java Sat Jun 25 00:06:20 2011
@@ -59,6 +59,7 @@ public class LogicalPlan extends BaseOpe
* graph has no correctness implications, whereas reversing the inputs
* of join can. This method of doing equals will detect predecessors
* in different orders but not successors in different orders.
+ * It will return false if either plan has non deterministic EvalFunc.
*/
@Override
public boolean isEqual(OperatorPlan other) throws FrontendException {
Modified: pig/trunk/src/org/apache/pig/newplan/logical/rules/OptimizerUtils.java
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/newplan/logical/rules/OptimizerUtils.java?rev=1139469&r1=1139468&r2=1139469&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/newplan/logical/rules/OptimizerUtils.java (original)
+++ pig/trunk/src/org/apache/pig/newplan/logical/rules/OptimizerUtils.java Sat Jun 25 00:06:20 2011
@@ -21,6 +21,7 @@ import java.util.Iterator;
import org.apache.pig.builtin.Nondeterministic;
import org.apache.pig.impl.PigContext;
+import org.apache.pig.impl.logicalLayer.FrontendException;
import org.apache.pig.newplan.Operator;
import org.apache.pig.newplan.logical.expression.LogicalExpressionPlan;
import org.apache.pig.newplan.logical.expression.UserFuncExpression;
@@ -75,16 +76,17 @@ public class OptimizerUtils {
*
* @param filterPlan
* @return true of the filter plan contains a non-deterministic UDF
+ * @throws FrontendException
*/
- public static boolean planHasNonDeterministicUdf(LogicalExpressionPlan filterPlan) {
+ public static boolean planHasNonDeterministicUdf(LogicalExpressionPlan filterPlan)
+ throws FrontendException {
Iterator<Operator> it = filterPlan.getOperators();
while( it.hasNext() ) {
Operator op = it.next();
if( op instanceof UserFuncExpression ) {
- Object udf = PigContext.instantiateFuncFromSpec(((UserFuncExpression) op).getFuncSpec());
- if (udf.getClass().getAnnotation(Nondeterministic.class) != null) {
+ if(! ((UserFuncExpression)op).isDeterministic() ){
return true;
-}
+ }
}
}
return false;
Modified: pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java
URL: http://svn.apache.org/viewvc/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java?rev=1139469&r1=1139468&r2=1139469&view=diff
==============================================================================
--- pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java (original)
+++ pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java Sat Jun 25 00:06:20 2011
@@ -23,8 +23,11 @@ import java.util.*;
import org.apache.pig.ExecType;
import org.apache.pig.PigServer;
import org.apache.pig.newplan.logical.optimizer.LogicalPlanOptimizer;
+import org.apache.pig.newplan.logical.relational.LOFilter;
import org.apache.pig.newplan.logical.relational.LogicalPlan;
+import org.apache.pig.newplan.logical.relational.LogicalRelationalOperator;
import org.apache.pig.newplan.logical.rules.LogicalExpressionSimplifier;
+import org.apache.pig.newplan.Operator;
import org.apache.pig.newplan.OperatorPlan;
import org.apache.pig.newplan.optimizer.PlanOptimizer;
import org.apache.pig.newplan.optimizer.Rule;
@@ -737,7 +740,33 @@ public class TestFilterSimplification ex
assertTrue(expected.isEqual(newLogicalPlan));
}
-
+
+
+ @Test
+ // PIG-2137
+ public void testSimiplificationNonDeterministicUdf() throws Exception {
+ String query = "b = filter (load 'd.txt' as (a0, a1)) by RANDOM() > 0.1 and RANDOM() > 0.1;" +
+ "store b into 'empty';";
+ LogicalPlan newLogicalPlan = Util.buildLp(pigServer, query);;
+
+ PlanOptimizer optimizer = new MyPlanOptimizer(newLogicalPlan, 10);
+ optimizer.optimize();
+ LOFilter optimizedFilt = (LOFilter) newLogicalPlan.getSuccessors(newLogicalPlan.getSources().get(0)).get(0);
+
+ //expected query is same as original query, optimizer should not combine
+ // conditions involving non deterministic udfs
+ query = "b = filter (load 'd.txt' as (a0, a1)) by RANDOM() > 0.1 and RANDOM() > 0.1;" +
+ "store b into 'empty';";
+ LogicalPlan expected = Util.buildLp(pigServer, query);;
+ LOFilter expectedFilt = (LOFilter) expected.getSuccessors(expected.getSources().get(0)).get(0);
+ assertEquals(
+ "size of filter expression plans",
+ optimizedFilt.getFilterPlan().size(),
+ expectedFilt.getFilterPlan().size()
+ );
+
+ }
+
public class MyPlanOptimizer extends LogicalPlanOptimizer {
protected MyPlanOptimizer(OperatorPlan p, int iterations) {