Author: yanz Date: Mon Sep 27 17:50:17 2010 New Revision: 1001838 URL: http://svn.apache.org/viewvc?rev=1001838&view=rev Log: PIG-1647: Logical simplifier throws a NPE (yanz)
Modified: hadoop/pig/trunk/CHANGES.txt hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.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=1001838&r1=1001837&r2=1001838&view=diff ============================================================================== --- hadoop/pig/trunk/CHANGES.txt (original) +++ hadoop/pig/trunk/CHANGES.txt Mon Sep 27 17:50:17 2010 @@ -207,6 +207,8 @@ PIG-1309: Map-side Cogroup (ashutoshc) BUG FIXES +PIG-1647: Logical simplifier throws a NPE (yanz) + PIG-1642: Order by doesn't use estimation to determine the parallelism (rding) PIG-1644: New logical plan: Plan.connect with position is misused in some Modified: hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java URL: http://svn.apache.org/viewvc/hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java?rev=1001838&r1=1001837&r2=1001838&view=diff ============================================================================== --- hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java (original) +++ hadoop/pig/trunk/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java Mon Sep 27 17:50:17 2010 @@ -147,20 +147,30 @@ class DNFPlanGenerator extends AllSameEx int lsize = lhsChildren.length, rsize = rhsChildren.length; LogicalExpression[][] grandChildrenL = new LogicalExpression[lsize][];; for (int i = 0; i < lsize; i++) { - if (lhsChildren[i] instanceof AndExpression || lhsChildren[i] instanceof DNFExpression) grandChildrenL[i] = dnfPlan.getSuccessors( - lhsChildren[i]).toArray( - new LogicalExpression[0]); - else { + if (lhsChildren[i] instanceof AndExpression) { + grandChildrenL[i] = lhsChildren[i].getPlan().getSuccessors( + lhsChildren[i]).toArray( + new LogicalExpression[0]); + } else if (lhsChildren[i] instanceof DNFExpression) { + grandChildrenL[i] = dnfPlan.getSuccessors( + lhsChildren[i]).toArray( + new LogicalExpression[0]); + } else { grandChildrenL[i] = new LogicalExpression[1]; grandChildrenL[i][0] = (LogicalExpression) lhsChildren[i]; } } LogicalExpression[][] grandChildrenR = new LogicalExpression[rsize][];; for (int i = 0; i < rsize; i++) { - if (rhsChildren[i] instanceof AndExpression || rhsChildren[i] instanceof DNFExpression) grandChildrenR[i] = dnfPlan.getSuccessors( - rhsChildren[i]).toArray( - new LogicalExpression[0]); - else { + if (rhsChildren[i] instanceof AndExpression) { + grandChildrenR[i] = rhsChildren[i].getPlan().getSuccessors( + rhsChildren[i]).toArray( + new LogicalExpression[0]); + } else if (rhsChildren[i] instanceof DNFExpression) { + grandChildrenR[i] = dnfPlan.getSuccessors( + rhsChildren[i]).toArray( + new LogicalExpression[0]); + } else { grandChildrenR[i] = new LogicalExpression[1]; grandChildrenR[i][0] = (LogicalExpression) rhsChildren[i]; } @@ -248,9 +258,14 @@ class DNFPlanGenerator extends AllSameEx int size = orChildren.length; LogicalExpression[][] grandChildrenOr = new LogicalExpression[size][];; for (int i = 0; i < size; i++) { - if (orChildren[i] instanceof AndExpression || orChildren[i] instanceof DNFExpression) grandChildrenOr[i] = dnfPlan.getSuccessors( + if (orChildren[i] instanceof DNFExpression) + grandChildrenOr[i] = dnfPlan.getSuccessors( orChildren[i]).toArray( new LogicalExpression[0]); + else if (orChildren[i] instanceof AndExpression) + grandChildrenOr[i] = orChildren[i].getPlan().getSuccessors( + orChildren[i]).toArray( + new LogicalExpression[0]); else { grandChildrenOr[i] = new LogicalExpression[1]; grandChildrenOr[i][0] = (LogicalExpression) orChildren[i]; @@ -302,9 +317,14 @@ class DNFPlanGenerator extends AllSameEx boolean andDNF = and.getPlan() == dnfPlan, orDNF = or.getPlan() == dnfPlan; LogicalExpression[][] grandChildrenOr = new LogicalExpression[orSize][];; for (int i = 0; i < orSize; i++) { - if (orChildren[i] instanceof AndExpression || orChildren[i] instanceof DNFExpression) grandChildrenOr[i] = dnfPlan.getSuccessors( + if (orChildren[i] instanceof DNFExpression) + grandChildrenOr[i] = dnfPlan.getSuccessors( orChildren[i]).toArray( new LogicalExpression[0]); + else if (orChildren[i] instanceof AndExpression) + grandChildrenOr[i] = orChildren[i].getPlan().getSuccessors( + orChildren[i]).toArray( + new LogicalExpression[0]); else { grandChildrenOr[i] = new LogicalExpression[1]; grandChildrenOr[i][0] = (LogicalExpression) orChildren[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=1001838&r1=1001837&r2=1001838&view=diff ============================================================================== --- hadoop/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java (original) +++ hadoop/pig/trunk/test/org/apache/pig/test/TestFilterSimplification.java Mon Sep 27 17:50:17 2010 @@ -666,6 +666,110 @@ public class TestFilterSimplification ex assertTrue(expected.isEqual(newLogicalPlan)); } + @Test + public void test4() throws Exception { + LogicalPlanTester lpt = new LogicalPlanTester(pc); + lpt.buildPlan("b = filter (load 'd.txt' as (a:chararray, b:long, c:map[], d:chararray, e:chararray)) by a == 'v' and b == 117L and c#'p1' == 'h' and c#'p2' == 'to' and ((d is not null and d != '') or (e is not null and e != ''));"); + + 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 (a:chararray, b:long, c:map[], d:chararray, e:chararray)) by a == 'v' and b == 117L and c#'p1' == 'h' and c#'p2' == 'to' and ((d is not null and d != '') or (e is not null and e != ''));"); + plan = lpt.buildPlan("store b into 'empty';"); + LogicalPlan expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + + // mirror of the above + lpt.buildPlan("b = filter (load 'd.txt' as (a:chararray, b:long, c:map[], d:chararray, e:chararray)) by ((d is not null and d != '') or (e is not null and e != '')) and a == 'v' and b == 117L and c#'p1' == 'h' and c#'p2' == 'to';"); + + 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 (a:chararray, b:long, c:map[], d:chararray, e:chararray)) by ((d is not null and d != '') or (e is not null and e != '')) and a == 'v' and b == 117L and c#'p1' == 'h' and c#'p2' == 'to';"); + plan = lpt.buildPlan("store b into 'empty';"); + expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + + } + + @Test + public void test5() throws Exception { + // 2-level combo: 8 possibilities + boolean[] booleans = {false, true}; + for (boolean b1 : booleans) + for (boolean b2 : booleans) + for (boolean b3 : booleans) + comboRunner2(b1, b2, b3); + } + + private void comboRunner2(boolean b1, boolean b2, boolean b3) throws Exception { + StringBuilder sb = new StringBuilder(); + sb.append("b = filter (load 'd.txt' as (a:int, b:int, c:int, d:int)) by (((a < 1) " + (b1 ? "and" : "or") + " (b < 2)) " + (b2 ? "and" : "or") + " ((c < 3) " + (b3 ? "and" : "or") + " (d < 4)));"); + String query = sb.toString(); + + LogicalPlanTester lpt = new LogicalPlanTester(pc); + lpt.buildPlan(query); + + 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(query); + plan = lpt.buildPlan("store b into 'empty';"); + LogicalPlan expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + } + + @Test + public void test6() throws Exception { + // 3-level combo: 128 possibilities + boolean[] booleans = {false, true}; + for (boolean b1 : booleans) + for (boolean b2 : booleans) + for (boolean b3 : booleans) + for (boolean b4 : booleans) + for (boolean b5 : booleans) + for (boolean b6 : booleans) + for (boolean b7 : booleans) + comboRunner3(b1, b2, b3, b4, b5, b6, b7); + } + + private void comboRunner3(boolean b1, boolean b2, boolean b3, boolean b4, boolean b5, boolean b6, boolean b7) throws Exception { + StringBuilder sb = new StringBuilder(); + sb.append("b = filter (load 'd.txt' as (a:int, b:int, c:int, d:int, e:int, f:int, g:int, h:int)) by ((((a < 1) " + (b1 ? "and" : "or") + " (b < 2)) " + (b2 ? "and" : "or") + " ((c < 3) " + (b3 ? "and" : "or") + " (d < 4))) " + (b4 ? "and" : "or") + " (((e < 5) " + (b5 ? "and" : "or") + " (f < 6)) " + (b6 ? "and" : "or") + " ((g < 7) " + (b7 ? "and" : "or") + " (h < 8))));"); + String query = sb.toString(); + + LogicalPlanTester lpt = new LogicalPlanTester(pc); + lpt.buildPlan(query); + + 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(query); + plan = lpt.buildPlan("store b into 'empty';"); + LogicalPlan expected = migratePlan(plan); + + assertTrue(expected.isEqual(newLogicalPlan)); + } + public class MyPlanOptimizer extends LogicalPlanOptimizer { protected MyPlanOptimizer(OperatorPlan p, int iterations) {