Author: yanz Date: Sat Sep 25 01:06:37 2010 New Revision: 1001117 URL: http://svn.apache.org/viewvc?rev=1001117&view=rev Log: PIG-1635: Logical simplifier does not simplify away constants under AND and OR; after simplificaion the ordering of operands of AND and OR may get changed (yanz)
Modified: hadoop/pig/branches/branch-0.8/CHANGES.txt hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java hadoop/pig/branches/branch-0.8/test/org/apache/pig/test/TestFilterSimplification.java Modified: hadoop/pig/branches/branch-0.8/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/pig/branches/branch-0.8/CHANGES.txt?rev=1001117&r1=1001116&r2=1001117&view=diff ============================================================================== --- hadoop/pig/branches/branch-0.8/CHANGES.txt (original) +++ hadoop/pig/branches/branch-0.8/CHANGES.txt Sat Sep 25 01:06:37 2010 @@ -198,6 +198,9 @@ PIG-1309: Map-side Cogroup (ashutoshc) BUG FIXES +PIG-1635: Logical simplifier does not simplify away constants under AND and OR; after simplificaion the ordering of operands of + AND and OR may get changed (yanz) + PIG-1639: New logical plan: PushUpFilter should not push before group/cogroup if filter condition contains UDF (xuefuz via daijy) Modified: hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java URL: http://svn.apache.org/viewvc/hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java?rev=1001117&r1=1001116&r2=1001117&view=diff ============================================================================== --- hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java (original) +++ hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java Sat Sep 25 01:06:37 2010 @@ -26,6 +26,7 @@ import org.apache.pig.newplan.ReverseDep import org.apache.pig.newplan.Operator; import org.apache.pig.newplan.OperatorPlan; import org.apache.pig.data.DataType; +import org.apache.pig.impl.util.Pair; /** * a constant expression evaluation visitor that will evaluate the constant expressions. @@ -244,8 +245,8 @@ class ConstExpEvaluator extends LogicalE Operator[] preds = predList.toArray(new Operator[0]); for (Object p : preds) { Operator pred = (Operator) p; - plan.disconnect(pred, parent); - plan.connect(pred, newExp); + Pair<Integer, Integer> pos = plan.disconnect(pred, parent); + plan.connect(pred, pos.first, newExp, pos.second); } } plan.remove(parent); Modified: hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java URL: http://svn.apache.org/viewvc/hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java?rev=1001117&r1=1001116&r2=1001117&view=diff ============================================================================== --- hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java (original) +++ hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java Sat Sep 25 01:06:37 2010 @@ -211,6 +211,10 @@ public class LogicalExpressionSimplifier byte relation; int size = children.size(); for (int i = 0; i < size; i++) { + if (children.get(i) instanceof ConstantExpression && ((Boolean) ((ConstantExpression) children.get(i)).getValue())) + decrDNFSplitCount((LogicalExpression) children.get(i)); + } + for (int i = 0; i < size; i++) { LogicalExpression child1 = (LogicalExpression) children.get(i); for (int j = i + 1; j < size; j++) { LogicalExpression child2 = (LogicalExpression) children.get(j); @@ -255,6 +259,10 @@ public class LogicalExpressionSimplifier Operator[] children = plan.getSuccessors(or).toArray( new Operator[0]); int size = children.length; + for (int i = 0; i < size; i++) { + if (children[i] instanceof ConstantExpression && !((Boolean) ((ConstantExpression) children[i]).getValue())) + decrDNFSplitCount((LogicalExpression) children[i]); + } for (int ii = 0; ii < size; ii++) { LogicalExpression child = (LogicalExpression) children[ii]; if (child instanceof AndExpression || (child instanceof DNFExpression && ((DNFExpression) child).type == DNFExpression.DNFExpressionType.AND)) { @@ -498,6 +506,9 @@ public class LogicalExpressionSimplifier private byte handleAndSimple(LogicalExpression e1, LogicalExpression e2) throws FrontendException { + if (e2 instanceof ConstantExpression) + // no relationship between an AND and a constant + return Unknown; // get the inference relation between e1, an AND expression, and e2, a leaf logical expression List<Operator> andChildren = e1.getPlan().getSuccessors(e1); boolean hasUnknown = false; Modified: hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java URL: http://svn.apache.org/viewvc/hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java?rev=1001117&r1=1001116&r2=1001117&view=diff ============================================================================== --- hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java (original) +++ hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java Sat Sep 25 01:06:37 2010 @@ -29,6 +29,7 @@ import org.apache.pig.newplan.Operator; import org.apache.pig.newplan.OperatorPlan; import org.apache.pig.newplan.PlanWalker; import org.apache.pig.newplan.PlanVisitor; +import org.apache.pig.impl.util.Pair; /** * A NOT conversion visitor that will traverse the expression tree in a depth-first @@ -59,8 +60,8 @@ class NOTConversionVisitor extends Logic if (p != null) { Operator[] preds = p.toArray(new Operator[0]); for (Operator pred : preds) { - plan.disconnect(pred, oldOp); - plan.connect(pred, newOp); + Pair<Integer, Integer> pos = plan.disconnect(pred, oldOp); + plan.connect(pred, pos.first, newOp, pos.second); } } List<Operator> s = plan.getSuccessors(oldOp); @@ -81,8 +82,8 @@ class NOTConversionVisitor extends Logic Operator[] preds = p.toArray(new Operator[0]); for (Operator pred : preds) { if (pred != before) { - plan.disconnect(pred, after); - plan.connect(pred, before); + Pair<Integer, Integer> pos = plan.disconnect(pred, after); + plan.connect(pred, pos.first, before, pos.second); } } } @@ -93,13 +94,12 @@ class NOTConversionVisitor extends Logic if (p != null) { Operator[] preds = p.toArray(new Operator[0]); for (Operator pred : preds) { - plan.disconnect(pred, op); + Pair<Integer, Integer> pos = plan.disconnect(pred, op); List<Operator> s = plan.getSuccessors(op); if (s != null) { Operator[] sucs = s.toArray(new Operator[0]); - for (Operator suc : sucs) { - plan.connect(pred, suc); - } + for (int i = 0; i < sucs.length; i++) + plan.connect(pred, pos.first+i, sucs[i], pos.second+i); } } } Modified: hadoop/pig/branches/branch-0.8/test/org/apache/pig/test/TestFilterSimplification.java URL: http://svn.apache.org/viewvc/hadoop/pig/branches/branch-0.8/test/org/apache/pig/test/TestFilterSimplification.java?rev=1001117&r1=1001116&r2=1001117&view=diff ============================================================================== --- hadoop/pig/branches/branch-0.8/test/org/apache/pig/test/TestFilterSimplification.java (original) +++ hadoop/pig/branches/branch-0.8/test/org/apache/pig/test/TestFilterSimplification.java Sat Sep 25 01:06:37 2010 @@ -33,6 +33,7 @@ import org.apache.pig.impl.plan.VisitorE import org.apache.pig.test.utils.LogicalPlanTester; import junit.framework.TestCase; +import org.junit.Test; public class TestFilterSimplification extends TestCase { @@ -48,6 +49,7 @@ public class TestFilterSimplification ex return newplan; } + @Test public void test1() throws Exception { // case 1: simple and implication LogicalPlanTester lpt = new LogicalPlanTester(pc); @@ -547,6 +549,7 @@ public class TestFilterSimplification ex } + @Test public void test2() throws Exception { LogicalPlanTester lpt = new LogicalPlanTester(pc); lpt.buildPlan("b = filter (load 'd.txt' as (name, age, gpa)) by age >= 50 or name > 'fred' and gpa <= 3.0 or name >= 'bob';"); @@ -612,6 +615,57 @@ public class TestFilterSimplification ex assertTrue(expected.isEqual(newLogicalPlan)); } + @Test + public void test3() throws Exception { + // boolean constant elimination: AND + LogicalPlanTester lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (id:int, v1, v2)) by ((v1 is not null) AND (id == 1) AND (1 == 1));"); + org.apache.pig.impl.logicalLayer.LogicalPlan plan = lpt.buildPlan("store b into 'empty';"); + LogicalPlan newLogicalPlan = migratePlan(plan); + + PlanOptimizer optimizer = new MyPlanOptimizer(newLogicalPlan, 10); + optimizer.optimize(); + + lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (id:int, v1, v2)) by ((v1 is not null) AND (id == 1));"); + plan = lpt.buildPlan("store b into 'empty';"); + LogicalPlan expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + + // boolean constant elimination: OR + lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (id:int, v1, v2)) by (((v1 is not null) AND (id == 1)) OR (1 == 0));"); + plan = lpt.buildPlan("store b into 'empty';"); + newLogicalPlan = migratePlan(plan); + + optimizer = new MyPlanOptimizer(newLogicalPlan, 10); + optimizer.optimize(); + + lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (id:int, v1, v2)) by ((v1 is not null) AND (id == 1));"); + plan = lpt.buildPlan("store b into 'empty';"); + expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + + // the mirror case of the above + lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (id:int, v1, v2)) by ((1 == 0) OR ((v1 is not null) AND (id == 1)));"); + plan = lpt.buildPlan("store b into 'empty';"); + newLogicalPlan = migratePlan(plan); + + optimizer = new MyPlanOptimizer(newLogicalPlan, 10); + optimizer.optimize(); + + lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (id:int, v1, v2)) by ((v1 is not null) AND (id == 1));"); + plan = lpt.buildPlan("store b into 'empty';"); + expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + } + public class MyPlanOptimizer extends LogicalPlanOptimizer { protected MyPlanOptimizer(OperatorPlan p, int iterations) {