Author: yanz
Date: Sat Sep 25 00:30:30 2010
New Revision: 1001113
URL: http://svn.apache.org/viewvc?rev=1001113&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/trunk/CHANGES.txt
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java
hadoop/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java
Modified: hadoop/pig/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/pig/trunk/CHANGES.txt?rev=1001113&r1=1001112&r2=1001113&view=diff
==============================================================================
--- hadoop/pig/trunk/CHANGES.txt (original)
+++ hadoop/pig/trunk/CHANGES.txt Sat Sep 25 00:30:30 2010
@@ -207,6 +207,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/trunk/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java
URL:
http://svn.apache.org/viewvc/hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java?rev=1001113&r1=1001112&r2=1001113&view=diff
==============================================================================
---
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java
(original)
+++
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java
Sat Sep 25 00:30:30 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/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java
URL:
http://svn.apache.org/viewvc/hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java?rev=1001113&r1=1001112&r2=1001113&view=diff
==============================================================================
---
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java
(original)
+++
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/LogicalExpressionSimplifier.java
Sat Sep 25 00:30:30 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/trunk/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java
URL:
http://svn.apache.org/viewvc/hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java?rev=1001113&r1=1001112&r2=1001113&view=diff
==============================================================================
---
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java
(original)
+++
hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/NotConversionVisitor.java
Sat Sep 25 00:30:30 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/trunk/test/org/apache/pig/test/TestFilterSimplification.java
URL:
http://svn.apache.org/viewvc/hadoop/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java?rev=1001113&r1=1001112&r2=1001113&view=diff
==============================================================================
--- hadoop/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java
(original)
+++ hadoop/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java Sat
Sep 25 00:30:30 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) {
