Author: yanz
Date: Mon Sep 27 17:56:52 2010
New Revision: 1001842

URL: http://svn.apache.org/viewvc?rev=1001842&view=rev
Log:
PIG-1647: Logical simplifier throws a NPE (yanz)

Modified:
    hadoop/pig/branches/branch-0.8/CHANGES.txt
    
hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.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=1001842&r1=1001841&r2=1001842&view=diff
==============================================================================
--- hadoop/pig/branches/branch-0.8/CHANGES.txt (original)
+++ hadoop/pig/branches/branch-0.8/CHANGES.txt Mon Sep 27 17:56:52 2010
@@ -198,6 +198,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/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java
URL: 
http://svn.apache.org/viewvc/hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java?rev=1001842&r1=1001841&r2=1001842&view=diff
==============================================================================
--- 
hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java
 (original)
+++ 
hadoop/pig/branches/branch-0.8/src/org/apache/pig/newplan/logical/rules/DNFPlanGenerator.java
 Mon Sep 27 17:56:52 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/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=1001842&r1=1001841&r2=1001842&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
 Mon Sep 27 17:56:52 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) {


Reply via email to