PHOENIX-2141 ComparisonExpression should return Boolean null if either operand is null
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/bbed18fd Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/bbed18fd Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/bbed18fd Branch: refs/heads/4.5-HBase-1.0 Commit: bbed18fdb53d214f7927c1881f1b12306d3932d9 Parents: 9227b32 Author: maryannxue <wei....@intel.com> Authored: Mon Jul 27 14:50:16 2015 -0400 Committer: James Taylor <jtay...@salesforce.com> Committed: Tue Aug 11 18:42:11 2015 -0700 ---------------------------------------------------------------------- .../phoenix/compile/ExpressionCompiler.java | 3 + .../apache/phoenix/compile/HavingCompiler.java | 2 +- .../apache/phoenix/compile/WhereCompiler.java | 2 +- .../apache/phoenix/compile/WhereOptimizer.java | 2 +- .../phoenix/expression/AndExpression.java | 3 + .../expression/ComparisonExpression.java | 4 +- .../phoenix/expression/LiteralExpression.java | 23 ++++++-- .../phoenix/expression/NullValueTest.java | 59 ++++++++++++++++++++ 8 files changed, 89 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java index 1278494..1523dce 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java @@ -251,6 +251,9 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio if (child.getDataType() != PBoolean.INSTANCE) { throw TypeMismatchException.newException(PBoolean.INSTANCE, child.getDataType(), child.toString()); } + if (LiteralExpression.isBooleanNull(child)) { + return child; + } if (LiteralExpression.isFalse(child)) { iterator.remove(); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/compile/HavingCompiler.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/HavingCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/HavingCompiler.java index 224a9b4..9ccd2f0 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/HavingCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/HavingCompiler.java @@ -54,7 +54,7 @@ public class HavingCompiler { if (expression.getDataType() != PBoolean.INSTANCE) { throw TypeMismatchException.newException(PBoolean.INSTANCE, expression.getDataType(), expression.toString()); } - if (LiteralExpression.isFalse(expression)) { + if (LiteralExpression.isBooleanFalseOrNull(expression)) { context.setScanRanges(ScanRanges.NOTHING); return null; } else if (LiteralExpression.isTrue(expression)) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereCompiler.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereCompiler.java index 9631850..13963d7 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereCompiler.java @@ -235,7 +235,7 @@ public class WhereCompiler { private static void setScanFilter(StatementContext context, FilterableStatement statement, Expression whereClause, boolean disambiguateWithFamily, boolean hashJoinOptimization) { Scan scan = context.getScan(); - if (LiteralExpression.isFalse(whereClause)) { + if (LiteralExpression.isBooleanFalseOrNull(whereClause)) { context.setScanRanges(ScanRanges.NOTHING); } else if (whereClause != null && !LiteralExpression.isTrue(whereClause) && !hashJoinOptimization) { Filter filter = null; http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java index 6dfae7e..601eee1 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java @@ -120,7 +120,7 @@ public class WhereOptimizer { context.setScanRanges(ScanRanges.EVERYTHING); return whereClause; } - if (LiteralExpression.isFalse(whereClause)) { + if (LiteralExpression.isBooleanFalseOrNull(whereClause)) { context.setScanRanges(ScanRanges.NOTHING); return null; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/expression/AndExpression.java ---------------------------------------------------------------------- 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..29b024d 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 @@ -44,6 +44,9 @@ public class AndExpression extends AndOrExpression { if (child.getDataType() != PBoolean.INSTANCE) { throw TypeMismatchException.newException(PBoolean.INSTANCE, child.getDataType(), child.toString()); } + if (LiteralExpression.isBooleanNull(child)) { + return child; + } if (LiteralExpression.isFalse(child)) { return child; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java index 1c8df20..074ac0a 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java @@ -142,7 +142,7 @@ public class ComparisonExpression extends BaseCompoundExpression { if (lhsExpr instanceof LiteralExpression) { lhsValue = ((LiteralExpression)lhsExpr).getValue(); if (lhsValue == null) { - return LiteralExpression.newConstant(false, PBoolean.INSTANCE, lhsExpr.getDeterminism()); + return LiteralExpression.newConstant(null, PBoolean.INSTANCE, lhsExpr.getDeterminism()); } } Object rhsValue = null; @@ -150,7 +150,7 @@ public class ComparisonExpression extends BaseCompoundExpression { if (rhsExpr instanceof LiteralExpression) { rhsValue = ((LiteralExpression)rhsExpr).getValue(); if (rhsValue == null) { - return LiteralExpression.newConstant(false, PBoolean.INSTANCE, rhsExpr.getDeterminism()); + return LiteralExpression.newConstant(null, PBoolean.INSTANCE, rhsExpr.getDeterminism()); } } if (lhsValue != null && rhsValue != null) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java index 05bd9b3..e911aae 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java @@ -89,10 +89,10 @@ public class LiteralExpression extends BaseTerminalExpression { } public static boolean isFalse(Expression child) { - if (child!=null) { - return child == BOOLEAN_EXPRESSIONS[child.getDeterminism().ordinal()]; - } - return false; + if (child!=null) { + return child == BOOLEAN_EXPRESSIONS[child.getDeterminism().ordinal()]; + } + return false; } public static boolean isTrue(Expression child) { @@ -101,6 +101,21 @@ public class LiteralExpression extends BaseTerminalExpression { } return false; } + + public static boolean isBooleanNull(Expression child) { + if (child!=null) { + return child == TYPED_NULL_EXPRESSIONS[PBoolean.INSTANCE.ordinal()+PDataType.values().length*child.getDeterminism().ordinal()]; + } + return false; + } + + public static boolean isBooleanFalseOrNull(Expression child) { + if (child!=null) { + return child == BOOLEAN_EXPRESSIONS[child.getDeterminism().ordinal()] + || child == TYPED_NULL_EXPRESSIONS[PBoolean.INSTANCE.ordinal()+PDataType.values().length*child.getDeterminism().ordinal()]; + } + return false; + } public static LiteralExpression newConstant(Object value) { return newConstant(value, Determinism.ALWAYS); http://git-wip-us.apache.org/repos/asf/phoenix/blob/bbed18fd/phoenix-core/src/test/java/org/apache/phoenix/expression/NullValueTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/NullValueTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/NullValueTest.java new file mode 100644 index 0000000..d5addf0 --- /dev/null +++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/NullValueTest.java @@ -0,0 +1,59 @@ +package org.apache.phoenix.expression; + +import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.util.Properties; + +import org.apache.phoenix.query.BaseConnectionlessQueryTest; +import org.apache.phoenix.util.PropertiesUtil; +import org.junit.Test; + +public class NullValueTest extends BaseConnectionlessQueryTest { + + @Test + public void testComparisonExpressionWithNullOperands() throws Exception { + String[] query = {"SELECT 'a' >= ''", + "SELECT '' < 'a'", + "SELECT '' = ''"}; + Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); + Connection conn = DriverManager.getConnection(getUrl(), props); + try { + for (String q : query) { + ResultSet rs = conn.createStatement().executeQuery(q); + assertTrue(rs.next()); + assertNull(rs.getObject(1)); + assertEquals(false, rs.getBoolean(1)); + assertFalse(rs.next()); + } + } finally { + conn.close(); + } + } + + @Test + public void testAndOrExpressionWithNullOperands() throws Exception { + String[] query = {"SELECT 'a' >= '' or '' < 'a'", + "SELECT 'a' >= '' and '' < 'a'"}; + Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); + Connection conn = DriverManager.getConnection(getUrl(), props); + try { + for (String q : query) { + ResultSet rs = conn.createStatement().executeQuery(q); + assertTrue(rs.next()); + assertNull(rs.getObject(1)); + assertEquals(false, rs.getBoolean(1)); + assertFalse(rs.next()); + } + } finally { + conn.close(); + } + } + +}
