You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pig.apache.org by ol...@apache.org on 2008/09/17 17:26:23 UTC

svn commit: r696341 - in /incubator/pig/branches/types: ./ src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/ test/org/apache/pig/test/

Author: olga
Date: Wed Sep 17 08:26:22 2008
New Revision: 696341

URL: http://svn.apache.org/viewvc?rev=696341&view=rev
Log:
PIG-434: short circuit AND and OR

Modified:
    incubator/pig/branches/types/CHANGES.txt
    incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POAnd.java
    incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POOr.java
    incubator/pig/branches/types/test/org/apache/pig/test/TestBoolean.java

Modified: incubator/pig/branches/types/CHANGES.txt
URL: http://svn.apache.org/viewvc/incubator/pig/branches/types/CHANGES.txt?rev=696341&r1=696340&r2=696341&view=diff
==============================================================================
--- incubator/pig/branches/types/CHANGES.txt (original)
+++ incubator/pig/branches/types/CHANGES.txt Wed Sep 17 08:26:22 2008
@@ -199,3 +199,5 @@
     
     PIG-429: Self join wth implicit split has the join output in wrong order
     (pradeepk via olgan)
+
+    PIG-434: short-circuit AND and OR (pradeepk viia olgan)

Modified: incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POAnd.java
URL: http://svn.apache.org/viewvc/incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POAnd.java?rev=696341&r1=696340&r2=696341&view=diff
==============================================================================
--- incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POAnd.java (original)
+++ incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POAnd.java Wed Sep 17 08:26:22 2008
@@ -60,21 +60,35 @@
     public Result getNext(Boolean b) throws ExecException {
         Result left;
         left = lhs.getNext(dummyBool);
-        if(left.returnStatus != POStatus.STATUS_OK || left.result == null) {
+        // pass on ERROR and EOP 
+        if(left.returnStatus != POStatus.STATUS_OK && left.returnStatus != POStatus.STATUS_NULL) {
             return left;
         }
         
-        // Cannot short circuit since rhs could be null and then we should
-        // be returning null.
+        // truth table for AND 
+        // t = true, n = null, f = false
+        //    AND  t n f
+        // 1) t    t n f
+        // 2) n    n n f
+        // 3) f    f f f
+        
+        // Short circuit - if lhs is false, return false; ROW 3 above is handled with this
+        if (left.result != null && !(((Boolean)left.result).booleanValue())) return left;
+        
         Result right = rhs.getNext(dummyBool);
-        if(right.returnStatus != POStatus.STATUS_OK || right.result == null) {
-        	return right;
+        // pass on ERROR and EOP 
+        if(right.returnStatus != POStatus.STATUS_OK && right.returnStatus != POStatus.STATUS_NULL) {
+            return right;
         }
         
-        if (!(((Boolean)left.result).booleanValue())) return left;
+        // if the lhs is null and rhs is true - return null, in all other cases
+        // we can just return the rhs - ROW 1 and ROW 2 of table above
+        if(left.result == null && right.result != null && ((Boolean)right.result).booleanValue()) {
+            return left;
+        }
         
         // No matter what, what we get from the right side is what we'll
-        // return, error, null, true, or false.
+        // return, null, true, or false.
         return right;
     }
 

Modified: incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POOr.java
URL: http://svn.apache.org/viewvc/incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POOr.java?rev=696341&r1=696340&r2=696341&view=diff
==============================================================================
--- incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POOr.java (original)
+++ incubator/pig/branches/types/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POOr.java Wed Sep 17 08:26:22 2008
@@ -60,21 +60,35 @@
     public Result getNext(Boolean b) throws ExecException {
         Result left;
         left = lhs.getNext(dummyBool);
-        if(left.returnStatus != POStatus.STATUS_OK || left.result == null) {
+        // pass on ERROR and EOP
+        if(left.returnStatus != POStatus.STATUS_OK && left.returnStatus != POStatus.STATUS_NULL) {
             return left;
         }
         
-        // Cannot short circuit since rhs could be null and then we should
-        // be returning null.
+        // truth table for OR 
+        // t = true, n = null, f = false
+        //    OR   t n f
+        // 1) t    t t t
+        // 2) n    t n n
+        // 3) f    t n f
+        
+        // Short circuit. if lhs is true, return true - ROW 1 above is handled with this
+        if (left.result != null && ((Boolean)left.result).booleanValue()) return left;
+        
         Result right = rhs.getNext(dummyBool);
-        if(right.returnStatus != POStatus.STATUS_OK || right.result == null) {
-        	return right;
+        // pass on ERROR and EOP 
+        if(right.returnStatus != POStatus.STATUS_OK && right.returnStatus != POStatus.STATUS_NULL) {
+            return right;
         }
         
-        if (((Boolean)left.result).booleanValue()) return left;
+        // if the lhs is null and rhs is false - return null , in all other cases, we can
+        // just return rhs - ROW 2 and ROW 3 above
+        if(left.result == null && right.result != null && !((Boolean)right.result).booleanValue()) {
+            return left;
+        }
         
         // No matter what, what we get from the right side is what we'll
-        // return, error, null, true, or false.
+        // return, null, true, or false.
         return right;
     }
 

Modified: incubator/pig/branches/types/test/org/apache/pig/test/TestBoolean.java
URL: http://svn.apache.org/viewvc/incubator/pig/branches/types/test/org/apache/pig/test/TestBoolean.java?rev=696341&r1=696340&r2=696341&view=diff
==============================================================================
--- incubator/pig/branches/types/test/org/apache/pig/test/TestBoolean.java (original)
+++ incubator/pig/branches/types/test/org/apache/pig/test/TestBoolean.java Wed Sep 17 08:26:22 2008
@@ -73,13 +73,26 @@
     	setupAnd();
     	Boolean[] testWith = new Boolean[] { false, true, null};
     	
+    	// truth table for AND 
+        // t = true, n = null, f = false
+        // AND  t n f
+        // t    t n f
+        // n    n n f
+        // f    f f f
+    	
     	// test with first operand set to null
     	for (int i = 0; i < testWith.length; i++) {
     		lt.setValue(null);
     		rt.setValue(testWith[i]);
 			Result res = bop.getNext(dummy);
 			assertEquals(POStatus.STATUS_OK, res.returnStatus);
-	        assertEquals(null, (Boolean)res.result);
+			if(testWith[i] != null && testWith[i] == false) {
+			    // if rhs is false, result is false
+			    assertEquals(new Boolean(false), (Boolean) res.result);
+			} else {
+			    // else result is null
+			    assertEquals(null, (Boolean)res.result);    
+			}
 		}
     	
     	// test with second operand set to null
@@ -88,7 +101,13 @@
 			rt.setValue(null);
 			Result res = bop.getNext(dummy);
 			assertEquals(POStatus.STATUS_OK, res.returnStatus);
-	        assertEquals(null, (Boolean)res.result);
+			if(testWith[i] != null && testWith[i] == false) {
+                // if lhs is false, result is false
+                assertEquals(new Boolean(false), (Boolean) res.result);
+            } else {
+                // else result is null
+                assertEquals(null, (Boolean)res.result);    
+            }
 		}
     }
     
@@ -96,6 +115,12 @@
     public void testOrNull() throws ExecException {
     	setupOr();
     	Boolean[] testWith = new Boolean[] { false, true, null};
+    	// truth table for OR 
+        // t = true, n = null, f = false
+        // OR   t n f
+        // t    t t t
+        // n    t n n
+        // f    t n f
     	
     	// test with first operand set to null
     	for (int i = 0; i < testWith.length; i++) {
@@ -103,7 +128,13 @@
     		rt.setValue(testWith[i]);
 			Result res = bop.getNext(dummy);
 			assertEquals(POStatus.STATUS_OK, res.returnStatus);
-	        assertEquals(null, (Boolean)res.result);
+			if(testWith[i] != null && testWith[i] == true) {
+                // if rhs is true, result is true
+                assertEquals(new Boolean(true), (Boolean) res.result);
+            } else {
+                // else result is null
+                assertEquals(null, (Boolean)res.result);    
+            }
 		}
     	
     	// test with second operand set to null
@@ -112,7 +143,13 @@
 			rt.setValue(null);
 			Result res = bop.getNext(dummy);
 			assertEquals(POStatus.STATUS_OK, res.returnStatus);
-	        assertEquals(null, (Boolean)res.result);
+			if(testWith[i] != null && testWith[i] == true) {
+                // if lhs is true, result is true
+                assertEquals(new Boolean(true), (Boolean) res.result);
+            } else {
+                // else result is null
+                assertEquals(null, (Boolean)res.result);    
+            }
 		}
     }