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);
+ }
}
}
