You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ra...@apache.org on 2019/08/26 16:31:08 UTC

[phoenix] branch 4.x-HBase-1.5 updated: PHOENIX-5136 Rows with null values inserted by UPSERT .. ON DUPLICATE KEY UPDATE are included in query results when they shouldn't be(Miles Spielberg)

This is an automated email from the ASF dual-hosted git repository.

rajeshbabu pushed a commit to branch 4.x-HBase-1.5
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/4.x-HBase-1.5 by this push:
     new a7bddf5  PHOENIX-5136 Rows with null values inserted by UPSERT .. ON DUPLICATE KEY UPDATE are included in query results when they shouldn't be(Miles Spielberg)
a7bddf5 is described below

commit a7bddf530ab53ed41ff5263a86f5e9212628612d
Author: Rajeshbabu Chintaguntla <Rajeshbabu Chintaguntla>
AuthorDate: Mon Aug 26 22:00:56 2019 +0530

    PHOENIX-5136 Rows with null values inserted by UPSERT .. ON DUPLICATE KEY UPDATE are included in query results when they shouldn't be(Miles Spielberg)
---
 .../apache/phoenix/end2end/OnDuplicateKeyIT.java   |  34 +++
 .../apache/phoenix/expression/AndExpression.java   |   2 +-
 .../apache/phoenix/expression/AndOrExpression.java |  25 +-
 .../phoenix/filter/BooleanExpressionFilter.java    |   5 +-
 .../phoenix/expression/AndExpressionTest.java      | 297 +++++++++++++++++++++
 .../phoenix/expression/OrExpressionTest.java       | 293 ++++++++++++++++++++
 6 files changed, 648 insertions(+), 8 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKeyIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKeyIT.java
index f1ee0e7..4782e57 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKeyIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKeyIT.java
@@ -580,6 +580,40 @@ public class OnDuplicateKeyIT extends ParallelStatsDisabledIT {
         }
     }
 
+    @Test
+    public void testRowsCreatedViaUpsertOnDuplicateKeyShouldNotBeReturnedInQueryIfNotMatched() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        String ddl = "create table " + tableName + "(pk varchar primary key, counter1 bigint, counter2 smallint)";
+        conn.createStatement().execute(ddl);
+        createIndex(conn, tableName);
+        // The data has to be specifically starting with null for the first counter to fail the test. If you reverse the values, the test passes.
+        String dml1 = "UPSERT INTO " + tableName + " VALUES('a',NULL,2) ON DUPLICATE KEY UPDATE " +
+                "counter1 = CASE WHEN (counter1 IS NULL) THEN NULL ELSE counter1 END, " +
+                "counter2 = CASE WHEN (counter1 IS NULL) THEN 2 ELSE counter2 END";
+        conn.createStatement().execute(dml1);
+        conn.commit();
+
+        String dml2 = "UPSERT INTO " + tableName + " VALUES('b',1,2) ON DUPLICATE KEY UPDATE " +
+                "counter1 = CASE WHEN (counter1 IS NULL) THEN 1 ELSE counter1 END, " +
+                "counter2 = CASE WHEN (counter1 IS NULL) THEN 2 ELSE counter2 END";
+        conn.createStatement().execute(dml2);
+        conn.commit();
+
+        // Using this statement causes the test to pass
+        //ResultSet rs = conn.createStatement().executeQuery("SELECT * FROM " + tableName + " WHERE counter2 = 2 AND counter1 = 1");
+        // This statement should be equivalent to the one above, but it selects both rows.
+        ResultSet rs = conn.createStatement().executeQuery("SELECT pk, counter1, counter2 FROM " + tableName + " WHERE counter2 = 2 AND (counter1 = 1 OR counter1 = 1)");
+        assertTrue(rs.next());
+        assertEquals("b",rs.getString(1));
+        assertEquals(1,rs.getLong(2));
+        assertEquals(2,rs.getLong(3));
+        assertFalse(rs.next());
+
+        conn.close();
+    }
+
 
 }
     
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/AndExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/AndExpression.java
index 70e94ca..2aa1827 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/AndExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/AndExpression.java
@@ -80,7 +80,7 @@ public class AndExpression extends AndOrExpression {
 
     @Override
     protected boolean isStopValue(Boolean value) {
-        return !Boolean.TRUE.equals(value);
+        return Boolean.FALSE.equals(value);
     }
 
     @Override
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/AndOrExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/AndOrExpression.java
index ea8c375..07b07a2 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/AndOrExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/AndOrExpression.java
@@ -24,7 +24,7 @@ import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.phoenix.schema.types.PBoolean;
 import org.apache.phoenix.schema.types.PDataType;
 import org.apache.phoenix.schema.tuple.Tuple;
-
+import org.apache.phoenix.util.ByteUtil;
 
 /**
  * 
@@ -36,6 +36,8 @@ import org.apache.phoenix.schema.tuple.Tuple;
 public abstract class AndOrExpression extends BaseCompoundExpression {
     // Remember evaluation of child expression for partial evaluation
     private BitSet partialEvalState;
+    // true if we have seen NULL as the value of some child expression
+    private boolean seenNull = false;
    
     public AndOrExpression() {
     }
@@ -56,34 +58,45 @@ public abstract class AndOrExpression extends BaseCompoundExpression {
         } else {
             partialEvalState.clear();
         }
+        seenNull = false;
         super.reset();
     }
     
     @Override
     public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
-        boolean isNull = false;
+        boolean childFailed = false;
         for (int i = 0; i < children.size(); i++) {
-            Expression child = children.get(i);
             // If partial state is available, then use that to know we've already evaluated this
             // child expression and do not need to do so again.
             if (partialEvalState == null || !partialEvalState.get(i)) {
+                Expression child = children.get(i);
                 // Call through to child evaluate method matching parent call to allow child to optimize
                 // evaluate versus getValue code path.
                 if (child.evaluate(tuple, ptr)) {
                     // Short circuit if we see our stop value
                     if (isStopValue((Boolean) PBoolean.INSTANCE.toObject(ptr, child.getDataType()))) {
                         return true;
-                    } else if (partialEvalState != null) {
+                    }
+                    if (ptr.getLength() == 0) {
+                        seenNull = true;
+                    }
+                    if (partialEvalState != null) {
                         partialEvalState.set(i);
                     }
                 } else {
-                    isNull = true;
+                    childFailed = true;
                 }
             }
         }
-        if (isNull) {
+        if (childFailed) {
             return false;
         }
+
+        if (seenNull) {
+            // Some child evaluated to NULL and we never saw a stop value.
+            // The expression evaluates as NULL even if the last child evaluated was non-NULL.
+            ptr.set(ByteUtil.EMPTY_BYTE_ARRAY);
+        }
         return true;
     }
 
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/BooleanExpressionFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/BooleanExpressionFilter.java
index 678ccca..80710c4 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/filter/BooleanExpressionFilter.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/BooleanExpressionFilter.java
@@ -96,7 +96,10 @@ abstract public class BooleanExpressionFilter extends FilterBase implements Writ
         } catch (IllegalDataException e) {
             return Boolean.FALSE;
         }
-        return (Boolean)expression.getDataType().toObject(tempPtr);
+        // If the entire Boolean expression evaluated to completion (evaluate returned true),
+        // but the result was SQL NULL, treat it as Java Boolean FALSE rather than returning null,
+        // which is used above to indicate incomplete evaluation.
+        return Boolean.TRUE.equals(expression.getDataType().toObject(tempPtr));
     }
 
     @Override
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/AndExpressionTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/AndExpressionTest.java
new file mode 100644
index 0000000..a8f1529
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/AndExpressionTest.java
@@ -0,0 +1,297 @@
+package org.apache.phoenix.expression;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.query.QueryConstants;
+import org.apache.phoenix.schema.PBaseColumn;
+import org.apache.phoenix.schema.PColumn;
+import org.apache.phoenix.schema.PName;
+import org.apache.phoenix.schema.PNameFactory;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.tuple.MultiKeyValueTuple;
+import org.apache.phoenix.schema.types.PBoolean;
+import org.apache.phoenix.schema.types.PDataType;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AndExpressionTest {
+
+    private AndExpression createAnd(Expression lhs, Expression rhs) {
+        return new AndExpression(Arrays.asList(lhs, rhs));
+    }
+
+    private AndExpression createAnd(Boolean x, Boolean y) {
+        return createAnd(LiteralExpression.newConstant(x), LiteralExpression.newConstant(y));
+    }
+
+    private void testImmediateSingle(Boolean expected, Boolean lhs, Boolean rhs) {
+        AndExpression and = createAnd(lhs, rhs);
+        ImmutableBytesWritable out = new ImmutableBytesWritable();
+        MultiKeyValueTuple tuple = new MultiKeyValueTuple();
+        boolean success = and.evaluate(tuple, out);
+        assertTrue(success);
+        assertEquals(expected, PBoolean.INSTANCE.toObject(out));
+    }
+
+    // Evaluating AND when values of both sides are known should immediately succeed
+    // and return the same result regardless of order.
+    private void testImmediate(Boolean expected, Boolean a, Boolean b) {
+        testImmediateSingle(expected, a, b);
+        testImmediateSingle(expected, b, a);
+    }
+
+    private PColumn pcolumn(final String name) {
+        return new PBaseColumn() {
+            @Override public PName getName() {
+                return PNameFactory.newName(name);
+            }
+
+            @Override public PDataType getDataType() {
+                return PBoolean.INSTANCE;
+            }
+
+            @Override public PName getFamilyName() {
+                return PNameFactory.newName(QueryConstants.DEFAULT_COLUMN_FAMILY);
+            }
+
+            @Override public int getPosition() {
+                return 0;
+            }
+
+            @Override public Integer getArraySize() {
+                return null;
+            }
+
+            @Override public byte[] getViewConstant() {
+                return new byte[0];
+            }
+
+            @Override public boolean isViewReferenced() {
+                return false;
+            }
+
+            @Override public String getExpressionStr() {
+                return null;
+            }
+
+            @Override public boolean isRowTimestamp() {
+                return false;
+            }
+
+            @Override public boolean isDynamic() {
+                return false;
+            }
+
+            @Override public byte[] getColumnQualifierBytes() {
+                return null;
+            }
+
+            @Override public long getTimestamp() {
+                return 0;
+            }
+
+            @Override public boolean isDerived() {
+                return false;
+            }
+
+            @Override public boolean isExcluded() {
+                return false;
+            }
+
+            @Override public SortOrder getSortOrder() {
+                return null;
+            }
+        };
+    }
+
+    private KeyValueColumnExpression kvExpr(final String name) {
+        return new KeyValueColumnExpression(pcolumn(name));
+    }
+
+    private Cell createCell(String name, Boolean value) {
+        byte[] valueBytes = value == null ? null : value ? PBoolean.TRUE_BYTES : PBoolean.FALSE_BYTES;
+        return CellUtil.createCell(
+            Bytes.toBytes("row"),
+            QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES,
+            Bytes.toBytes(name),
+            1,
+            KeyValue.Type.Put.getCode(),
+            valueBytes);
+    }
+
+    private void testPartialOneSideFirst(Boolean expected, Boolean lhs, Boolean rhs, boolean leftFirst) {
+        KeyValueColumnExpression lhsExpr = kvExpr("LHS");
+        KeyValueColumnExpression rhsExpr = kvExpr("RHS");
+        AndExpression and = createAnd(lhsExpr, rhsExpr);
+        MultiKeyValueTuple tuple = new MultiKeyValueTuple(Collections.<Cell>emptyList());
+        ImmutableBytesWritable out = new ImmutableBytesWritable();
+
+        // with no data available, should fail
+        boolean success = and.evaluate(tuple, out);
+        assertFalse(success);
+
+        // with 1 datum available, should fail
+        if (leftFirst) {
+            tuple.setKeyValues(Collections.singletonList(createCell("LHS", lhs)));
+        } else {
+            tuple.setKeyValues(Collections.singletonList(createCell("RHS", rhs)));
+        }
+        success = and.evaluate(tuple, out);
+        assertFalse(success);
+
+        // with 2 data available, should succeed
+        tuple.setKeyValues(Arrays.asList(createCell("LHS", lhs), createCell("RHS", rhs)));
+        success = and.evaluate(tuple, out);
+        assertTrue(success);
+        assertEquals(expected, PBoolean.INSTANCE.toObject(out));
+    }
+
+    private void testPartialEvaluation(Boolean expected, Boolean x, Boolean y, boolean xFirst) {
+        testPartialOneSideFirst(expected, x, y, xFirst);
+        testPartialOneSideFirst(expected, y, x, !xFirst);
+    }
+
+    private void testShortCircuitOneSideFirst(Boolean expected, Boolean lhs, Boolean rhs, boolean leftFirst) {
+        KeyValueColumnExpression lhsExpr = kvExpr("LHS");
+        KeyValueColumnExpression rhsExpr = kvExpr("RHS");
+        AndExpression and = createAnd(lhsExpr, rhsExpr);
+        MultiKeyValueTuple tuple = new MultiKeyValueTuple(Collections.<Cell>emptyList());
+        ImmutableBytesWritable out = new ImmutableBytesWritable();
+
+        // with no data available, should fail
+        boolean success = and.evaluate(tuple, out);
+        assertFalse(success);
+
+        // with 1 datum available, should succeed
+        if (leftFirst) {
+            tuple.setKeyValues(Collections.singletonList(createCell("LHS", lhs)));
+        } else {
+            tuple.setKeyValues(Collections.singletonList(createCell("RHS", rhs)));
+        }
+        success = and.evaluate(tuple, out);
+        assertTrue(success);
+        assertEquals(expected, PBoolean.INSTANCE.toObject(out));
+    }
+
+
+    private void testShortCircuit(Boolean expected, Boolean x, Boolean y, boolean xFirst) {
+        testShortCircuitOneSideFirst(expected, x, y, xFirst);
+        testShortCircuitOneSideFirst(expected, y, x, !xFirst);
+    }
+
+    @Test
+    public void testImmediateCertainty() {
+        testImmediate(true, true, true);
+        testImmediate(false, false, true);
+        testImmediate(false, false, false);
+    }
+
+    @Test
+    public void testImmediateUncertainty() {
+        testImmediate(null, true, null);
+        testImmediate(false, false, null);
+        testImmediate(null, null, null);
+    }
+
+    @Test
+    public void testPartialCertainty() {
+        // T AND T = T
+        // must evaluate both sides, regardless of order
+        testPartialEvaluation(true, true, true, true);
+        testPartialEvaluation(true, true, true, false);
+
+        // T AND F = F
+        // must evaluate both sides if TRUE is evaluated first
+        testPartialEvaluation(false, true, false, true);
+        testPartialEvaluation(false, false, true, false);
+    }
+
+    @Test
+    public void testPartialUncertainty() {
+        // T AND NULL = NULL
+        // must evaluate both sides, regardless of order of values or evaluation
+        testPartialEvaluation(null, true, null, true);
+        testPartialEvaluation(null, true, null, false);
+        testPartialEvaluation(null, null, true, true);
+        testPartialEvaluation(null, null, true, false);
+
+        // must evaluate both sides if NULL is evaluated first
+
+        // F AND NULL = FALSE
+        testPartialEvaluation(false, null, false, true);
+        testPartialEvaluation(false, false, null, false);
+
+        // NULL AND NULL = NULL
+        testPartialEvaluation(null, null, null, true);
+        testPartialEvaluation(null, null, null, false);
+    }
+
+    @Test
+    public void testShortCircuitCertainty() {
+        // need only to evaluate one side if FALSE is evaluated first
+
+        // F AND F = F
+        testShortCircuit(false, false, false, true);
+        testShortCircuit(false, false, false, false);
+
+        // T AND F = F
+        testShortCircuit(false, false, true, true);
+        testShortCircuit(false, true, false, false);
+    }
+
+    @Test
+    public void testShortCircuitUncertainty() {
+        // need only to evaluate one side if FALSE is evaluated first
+
+        // F AND NULL = FALSE
+        testShortCircuit(false, false, null, true);
+        testShortCircuit(false, null, false, false);
+    }
+
+    @Test
+    public void testTruthTable() {
+        // See: https://en.wikipedia.org/wiki/Null_(SQL)#Comparisons_with_NULL_and_the_three-valued_logic_(3VL)
+        Boolean[][] testCases = new Boolean[][] {
+                //              should short circuit?
+                //    X,     Y, if X first, if Y first, X AND Y,
+                {  true,  true,      false,      false,    true, },
+                {  true, false,      false,       true,   false, },
+                { false, false,       true,       true,   false, },
+                {  true,  null,      false,      false,    null, },
+                { false,  null,       true,      false,   false, },
+                {  null,  null,      false,      false,    null, },
+        };
+
+        for (Boolean[] testCase : testCases) {
+            Boolean x = testCase[0];
+            Boolean y = testCase[1];
+            boolean shouldShortCircuitWhenXEvaluatedFirst = testCase[2];
+            boolean shouldShortCircuitWhenYEvaluatedFirst = testCase[3];
+            Boolean expected = testCase[4];
+
+            // test both directions
+            testImmediate(expected, x, y);
+
+            if (shouldShortCircuitWhenXEvaluatedFirst) {
+                testShortCircuit(expected, x, y, true);
+            } else {
+                testPartialEvaluation(expected, x, y, true);
+            }
+
+            if (shouldShortCircuitWhenYEvaluatedFirst) {
+                testShortCircuit(expected, x, y, false);
+            } else {
+                testPartialEvaluation(expected, x, y, false);
+            }
+        }
+    }
+}
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/OrExpressionTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/OrExpressionTest.java
new file mode 100644
index 0000000..7ab7a6d
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/OrExpressionTest.java
@@ -0,0 +1,293 @@
+package org.apache.phoenix.expression;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.query.QueryConstants;
+import org.apache.phoenix.schema.PBaseColumn;
+import org.apache.phoenix.schema.PColumn;
+import org.apache.phoenix.schema.PName;
+import org.apache.phoenix.schema.PNameFactory;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.tuple.MultiKeyValueTuple;
+import org.apache.phoenix.schema.types.PBoolean;
+import org.apache.phoenix.schema.types.PDataType;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class OrExpressionTest {
+
+    private OrExpression createOr(Expression lhs, Expression rhs) {
+        return new OrExpression(Arrays.asList(lhs, rhs));
+    }
+
+    private OrExpression createOr(Boolean x, Boolean y) {
+        return createOr(LiteralExpression.newConstant(x), LiteralExpression.newConstant(y));
+    }
+
+    private void testImmediateSingle(Boolean expected, Boolean lhs, Boolean rhs) {
+        OrExpression or = createOr(lhs, rhs);
+        ImmutableBytesWritable out = new ImmutableBytesWritable();
+        MultiKeyValueTuple tuple = new MultiKeyValueTuple();
+        boolean success = or.evaluate(tuple, out);
+        assertTrue(success);
+        assertEquals(expected, PBoolean.INSTANCE.toObject(out));
+    }
+
+    // Evaluating OR when values of both sides are known should immediately succeed
+    // and return the same result regardless of order.
+    private void testImmediate(Boolean expected, Boolean a, Boolean b) {
+        testImmediateSingle(expected, a, b);
+        testImmediateSingle(expected, b, a);
+    }
+
+    private PColumn pcolumn(final String name) {
+        return new PBaseColumn() {
+            @Override public PName getName() {
+                return PNameFactory.newName(name);
+            }
+
+            @Override public PDataType getDataType() {
+                return PBoolean.INSTANCE;
+            }
+
+            @Override public PName getFamilyName() {
+                return PNameFactory.newName(QueryConstants.DEFAULT_COLUMN_FAMILY);
+            }
+
+            @Override public int getPosition() {
+                return 0;
+            }
+
+            @Override public Integer getArraySize() {
+                return null;
+            }
+
+            @Override public byte[] getViewConstant() {
+                return new byte[0];
+            }
+
+            @Override public boolean isViewReferenced() {
+                return false;
+            }
+
+            @Override public String getExpressionStr() {
+                return null;
+            }
+
+            @Override public boolean isRowTimestamp() {
+                return false;
+            }
+
+            @Override public boolean isDynamic() {
+                return false;
+            }
+
+            @Override public byte[] getColumnQualifierBytes() {
+                return null;
+            }
+
+            @Override public long getTimestamp() {
+                return 0;
+            }
+
+            @Override public boolean isDerived() {
+                return false;
+            }
+
+            @Override public boolean isExcluded() {
+                return false;
+            }
+
+            @Override public SortOrder getSortOrder() {
+                return null;
+            }
+        };
+    }
+
+    private KeyValueColumnExpression kvExpr(final String name) {
+        return new KeyValueColumnExpression(pcolumn(name));
+    }
+
+    private Cell createCell(String name, Boolean value) {
+        byte[] valueBytes = value == null ? null : value ? PBoolean.TRUE_BYTES : PBoolean.FALSE_BYTES;
+        return CellUtil.createCell(
+                Bytes.toBytes("row"),
+                QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES,
+                Bytes.toBytes(name),
+                1,
+                KeyValue.Type.Put.getCode(),
+                valueBytes);
+    }
+
+    private void testPartialOneSideFirst(Boolean expected, Boolean lhs, Boolean rhs, boolean leftFirst) {
+        KeyValueColumnExpression lhsExpr = kvExpr("LHS");
+        KeyValueColumnExpression rhsExpr = kvExpr("RHS");
+        OrExpression or = createOr(lhsExpr, rhsExpr);
+        MultiKeyValueTuple tuple = new MultiKeyValueTuple(Collections.<Cell>emptyList());
+        ImmutableBytesWritable out = new ImmutableBytesWritable();
+
+        // with no data available, should fail
+        boolean success = or.evaluate(tuple, out);
+        assertFalse(success);
+
+        // with 1 datum available, should fail
+        if (leftFirst) {
+            tuple.setKeyValues(Collections.singletonList(createCell("LHS", lhs)));
+        } else {
+            tuple.setKeyValues(Collections.singletonList(createCell("RHS", rhs)));
+        }
+        success = or.evaluate(tuple, out);
+        assertFalse(success);
+
+        // with 2 data available, should succeed
+        tuple.setKeyValues(Arrays.asList(createCell("LHS", lhs), createCell("RHS", rhs)));
+        success = or.evaluate(tuple, out);
+        assertTrue(success);
+        assertEquals(expected, PBoolean.INSTANCE.toObject(out));
+    }
+
+    private void testPartialEvaluation(Boolean expected, Boolean x, Boolean y, boolean xFirst) {
+        testPartialOneSideFirst(expected, x, y, xFirst);
+        testPartialOneSideFirst(expected, y, x, !xFirst);
+    }
+
+    private void testShortCircuitOneSideFirst(Boolean expected, Boolean lhs, Boolean rhs, boolean leftFirst) {
+        KeyValueColumnExpression lhsExpr = kvExpr("LHS");
+        KeyValueColumnExpression rhsExpr = kvExpr("RHS");
+        OrExpression or = createOr(lhsExpr, rhsExpr);
+        MultiKeyValueTuple tuple = new MultiKeyValueTuple(Collections.<Cell>emptyList());
+        ImmutableBytesWritable out = new ImmutableBytesWritable();
+
+        // with no data available, should fail
+        boolean success = or.evaluate(tuple, out);
+        assertFalse(success);
+
+        // with 1 datum available, should succeed
+        if (leftFirst) {
+            tuple.setKeyValues(Collections.singletonList(createCell("LHS", lhs)));
+        } else {
+            tuple.setKeyValues(Collections.singletonList(createCell("RHS", rhs)));
+        }
+        success = or.evaluate(tuple, out);
+        assertTrue(success);
+        assertEquals(expected, PBoolean.INSTANCE.toObject(out));
+    }
+
+
+    private void testShortCircuit(Boolean expected, Boolean x, Boolean y, boolean xFirst) {
+        testShortCircuitOneSideFirst(expected, x, y, xFirst);
+        testShortCircuitOneSideFirst(expected, y, x, !xFirst);
+    }
+
+    @Test
+    public void testImmediateCertainty() {
+        testImmediate(true, true, true);
+        testImmediate(true, false, true);
+        testImmediate(false, false, false);
+    }
+
+    @Test
+    public void testImmediateUncertainty() {
+        testImmediate(true, true, null);
+        testImmediate(null, false, null);
+        testImmediate(null, null, null);
+    }
+
+    @Test
+    public void testPartialCertainty() {
+        // must evaluate both sides if FALSE is evaluated first
+
+        // F OR F = F
+        testPartialEvaluation(false, false, false, true);
+        testPartialEvaluation(false, false, false, false);
+
+        // T OR F = T
+        testPartialEvaluation(true, false, true, true);
+        testPartialEvaluation(true, true, false, false);
+    }
+
+    @Test
+    public void testPartialUncertainty() {
+        // T OR NULL = NULL
+        testPartialEvaluation(true, null, true, true);
+        testPartialEvaluation(true, true, null, false);
+
+        // must evaluate both sides if NULL is evaluated first
+
+        // F OR NULL = NULL
+        testPartialEvaluation(null, null, false, true);
+        testPartialEvaluation(null, false, null, false);
+
+        // NULL OR NULL = NULL
+        testPartialEvaluation(null, null, null, true);
+        testPartialEvaluation(null, null, null, false);
+    }
+
+    @Test
+    public void testShortCircuitCertainty() {
+        // need only to evaluate one side if TRUE is evaluated first
+
+        // T OR T = T
+        testShortCircuit(true, true, true, true);
+        testShortCircuit(true, true, true, false);
+
+
+        // T OR F = F
+        testShortCircuit(true, true, false, true);
+        testShortCircuit(true, false, true, false);
+    }
+
+    @Test
+    public void testShortCircuitUncertainty() {
+        // need only to evaluate one side if TRUE is evaluated first
+        testShortCircuit(true, true, null, true);
+        testShortCircuit(true, null, true, false);
+    }
+
+    @Test
+    public void testTruthTable() {
+        // See: https://en.wikipedia.org/wiki/Null_(SQL)#Comparisons_with_NULL_and_the_three-valued_logic_(3VL)
+        Boolean[][] testCases = new Boolean[][] {
+            //              should short circuit?
+            //    X,     Y, if X first, if Y first, X OR Y,
+            {  true,  true,       true,       true,   true, },
+            {  true, false,       true,      false,   true, },
+            { false, false,      false,      false,  false, },
+            {  true,  null,       true,      false,   true, },
+            { false,  null,      false,      false,   null, },
+            {  null,  null,      false,      false,   null, },
+        };
+
+        for (Boolean[] testCase : testCases) {
+            Boolean x = testCase[0];
+            Boolean y = testCase[1];
+            boolean shouldShortCircuitWhenXEvaluatedFirst = testCase[2];
+            boolean shouldShortCircuitWhenYEvaluatedFirst = testCase[3];
+            Boolean expected = testCase[4];
+
+            // test both directions
+            testImmediate(expected, x, y);
+
+            if (shouldShortCircuitWhenXEvaluatedFirst) {
+                testShortCircuit(expected, x, y, true);
+            } else {
+                testPartialEvaluation(expected, x, y, true);
+            }
+
+            if (shouldShortCircuitWhenYEvaluatedFirst) {
+                testShortCircuit(expected, x, y, false);
+            } else {
+                testPartialEvaluation(expected, x, y, false);
+            }
+        }
+    }
+}