You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pig.apache.org by da...@apache.org on 2011/05/13 22:38:20 UTC

svn commit: r1102892 - 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: daijy
Date: Fri May 13 20:38:20 2011
New Revision: 1102892

URL: http://svn.apache.org/viewvc?rev=1102892&view=rev
Log:
PIG-2067: FilterLogicExpressionSimplifier removed some branches in some cases

Modified:
    pig/trunk/CHANGES.txt
    pig/trunk/src/org/apache/pig/newplan/logical/expression/ProjectExpression.java
    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/LogicalExpressionSimplifier.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=1102892&r1=1102891&r2=1102892&view=diff
==============================================================================
--- pig/trunk/CHANGES.txt (original)
+++ pig/trunk/CHANGES.txt Fri May 13 20:38:20 2011
@@ -593,6 +593,8 @@ PIG-1309: Map-side Cogroup (ashutoshc)
 
 BUG FIXES
 
+PIG-2067: FilterLogicExpressionSimplifier removed some branches in some cases (daijy)
+
 PIG-2033: Pig returns sucess for the failed Pig script (rding)
 
 PIG-1993: PigStorageSchema throw NPE with ColumnPruning (daijy)

Modified: pig/trunk/src/org/apache/pig/newplan/logical/expression/ProjectExpression.java
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/newplan/logical/expression/ProjectExpression.java?rev=1102892&r1=1102891&r2=1102892&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/newplan/logical/expression/ProjectExpression.java (original)
+++ pig/trunk/src/org/apache/pig/newplan/logical/expression/ProjectExpression.java Fri May 13 20:38:20 2011
@@ -404,7 +404,18 @@ public class ProjectExpression extends C
     public boolean isEqual(Operator other) throws FrontendException {
         if (other != null && other instanceof ProjectExpression) {
             ProjectExpression po = (ProjectExpression)other;
-            return po.input == input && po.col == col;
+            if (po.input != input || po.col != col)
+                return false;
+            
+            Operator mySucc = getPlan().getSuccessors(this)!=null?
+                    getPlan().getSuccessors(this).get(0):null;
+            Operator theirSucc = other.getPlan().getSuccessors(other)!=null?
+                    other.getPlan().getSuccessors(other).get(0):null;
+            if (mySucc!=null && theirSucc!=null)
+                return mySucc.isEqual(theirSucc);
+            if (mySucc==null && theirSucc==null)
+                return true;
+            return false;
         } else {
             return false;
         }

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=1102892&r1=1102891&r2=1102892&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 Fri May 13 20:38:20 2011
@@ -69,7 +69,18 @@ public class UserFuncExpression extends 
     public boolean isEqual(Operator other) throws FrontendException {
         if( other instanceof UserFuncExpression ) {
             UserFuncExpression exp = (UserFuncExpression)other;
-            return mFuncSpec.equals(exp.mFuncSpec );
+            if (!mFuncSpec.equals(exp.mFuncSpec ))
+                return false;
+            
+            List<Operator> mySuccs = getPlan().getSuccessors(this);
+            List<Operator> theirSuccs = other.getPlan().getSuccessors(other);
+            if (mySuccs.size()!=theirSuccs.size())
+                return false;
+            for (int i=0;i<mySuccs.size();i++) {
+                if (!mySuccs.get(i).isEqual(theirSuccs.get(i)))
+                    return false;
+            }
+            return true;
         } else {
             return false;
         }

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=1102892&r1=1102891&r2=1102892&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 Fri May 13 20:38:20 2011
@@ -76,13 +76,12 @@ public class LogicalPlan extends BaseOpe
         ps.println("# New Logical Plan:");
         ps.println("#-----------------------------------------------");
 
-        if (format.equals("text")) {
-            LogicalPlanPrinter npp = new LogicalPlanPrinter(this, ps);
-            npp.visit();
-        } else if (format.equals("dot")) {
+        if (format.equals("dot")) {
             DotLOPrinter lpp = new DotLOPrinter(this, ps);
             lpp.dump();
-            ps.println("");
+        } else {
+            LogicalPlanPrinter npp = new LogicalPlanPrinter(this, ps);
+            npp.visit();
         }
     }
 

Modified: pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java?rev=1102892&r1=1102891&r2=1102892&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java (original)
+++ pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java Fri May 13 20:38:20 2011
@@ -18,6 +18,7 @@
 
 package org.apache.pig.newplan.logical.rules;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Iterator;
 
@@ -41,6 +42,8 @@ import org.apache.pig.newplan.optimizer.
 
 public class LogicalExpressionSimplifier extends Rule {
 
+    private List<LOFilter> processedFilters = new ArrayList<LOFilter>();
+    
     enum DNFExpressionType {
         AND, OR
     }
@@ -51,15 +54,27 @@ public class LogicalExpressionSimplifier
 
     @Override
     public Transformer getNewTransformer() {
-        return new LogicalExpressionSimplifierTransformer();
+        return new LogicalExpressionSimplifierTransformer(processedFilters);
     }
 
     public static class LogicalExpressionSimplifierTransformer extends Transformer {
         private static final String dnfCountAnnotationKey = "dnfSplitCount";
         private OperatorPlan plan;
+        
+        private List<LOFilter> processedFilters;
 
+        public LogicalExpressionSimplifierTransformer(List<LOFilter> processedFilters) {
+            this.processedFilters = processedFilters;
+        }
         @Override
         public boolean check(OperatorPlan matched) throws FrontendException {
+            LOFilter filter = (LOFilter)matched.getOperators().next();
+            
+            // If the filter is already processed, we quit.
+            if (processedFilters.contains(filter))
+                return false;
+            
+            processedFilters.add(filter);
             return true;
         }
 

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=1102892&r1=1102891&r2=1102892&view=diff
==============================================================================
--- pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java (original)
+++ pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java Fri May 13 20:38:20 2011
@@ -479,6 +479,28 @@ public class TestFilterSimplification ex
         expected = Util.buildLp(pigServer, query);;
 
         assertTrue(expected.isEqual(newLogicalPlan));
+        
+        // case 31: See PIG-2067
+        query = "A = load 'a.dat' as (cookie);" +
+                "B = load 'b.dat' as (cookie);" +
+                "C = cogroup A by cookie, B by cookie;" +
+                "E = filter C by COUNT(B)>0 AND COUNT(A)>0;" +
+                "store E into 'empty';";
+        newLogicalPlan = Util.buildLp(pigServer, query);;
+
+        optimizer = new MyPlanOptimizer(newLogicalPlan, 10);
+        optimizer.optimize();
+
+        // Make sure in this case, we don't optimize
+        query = "A = load 'a.dat' as (cookie);" +
+            "B = load 'b.dat' as (cookie);" +
+            "C = cogroup A by cookie, B by cookie;" +
+            "E = filter C by COUNT(B)>0 AND COUNT(A)>0;" +
+            "store E into 'empty';";
+        
+        expected = Util.buildLp(pigServer, query);;
+
+        assertTrue(expected.isEqual(newLogicalPlan));
 
     }