You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by st...@apache.org on 2020/03/18 05:15:21 UTC

[phoenix] branch master updated: PHOENIX-5065 Inconsistent treatment of NULL and empty string

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

stoty pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new ba609be  PHOENIX-5065 Inconsistent treatment of NULL and empty string
ba609be is described below

commit ba609bef5110c8a04a1cc3d7c60485fb8c23a1dc
Author: Richard Antal <an...@gmail.com>
AuthorDate: Mon Mar 9 14:53:53 2020 +0100

    PHOENIX-5065 Inconsistent treatment of NULL and empty string
    
    Signed-off-by: Istvan Toth <st...@apache.org>
---
 .../java/org/apache/phoenix/end2end/InListIT.java  | 49 ++++++++++++++++++++++
 .../phoenix/expression/InListExpression.java       | 44 ++++++++++++++-----
 2 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
index 2c5cf6a..7e0a7a5 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
@@ -35,6 +35,7 @@ import java.util.List;
 import java.util.Properties;
 
 import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.TypeMismatchException;
 import org.apache.phoenix.schema.types.PDataType;
 import org.apache.phoenix.schema.types.PInteger;
 import org.apache.phoenix.schema.types.PLong;
@@ -485,6 +486,54 @@ public class InListIT extends ParallelStatsDisabledIT {
         
         conn.close();
     }
+
+    @Test
+    public void testInListExpressionWithNull() throws Exception {
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            Statement stmt = conn.createStatement();
+            ResultSet rs = stmt.executeQuery("SELECT COUNT(*) FROM SYSTEM.CATALOG WHERE " +
+                    "TENANT_ID IN ('', 'FOO')");
+            ResultSet rs2 = stmt.executeQuery("SELECT COUNT(*) FROM SYSTEM.CATALOG WHERE " +
+                    "TENANT_ID = '' OR TENANT_ID = 'FOO'");
+            assertTrue(rs.next());
+            assertTrue(rs2.next());
+            assertEquals(rs.getInt(1), rs2.getInt(1));
+            assertEquals(0, rs.getInt(1));
+        }
+    }
+
+    @Test(expected = TypeMismatchException.class)
+    public void testInListExpressionWithNotValidElements() throws Exception {
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            Statement stmt = conn.createStatement();
+            ResultSet rs = stmt.executeQuery("SELECT COUNT(*) FROM SYSTEM.CATALOG WHERE " +
+                    "TENANT_ID IN (4, 8)");
+            assertTrue(rs.next());
+            assertEquals(0, rs.getInt(1));
+        }
+    }
+
+    @Test(expected = SQLException.class)
+    public void testInListExpressionWithNoElements() throws Exception {
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            Statement stmt = conn.createStatement();
+            ResultSet rs = stmt.executeQuery("SELECT COUNT(*) FROM SYSTEM.CATALOG WHERE " +
+                    "TENANT_ID IN ()");
+            assertTrue(rs.next());
+            assertEquals(0, rs.getInt(1));
+        }
+    }
+
+    @Test
+    public void testInListExpressionWithNullAndWrongTypedData() throws Exception {
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            Statement stmt = conn.createStatement();
+            ResultSet rs = stmt.executeQuery("SELECT COUNT(*) FROM SYSTEM.CATALOG WHERE " +
+                    "TENANT_ID IN ('', 4)");
+            assertTrue(rs.next());
+            assertEquals(0, rs.getInt(1));
+        }
+    }
     
     @Test
     public void testInListExpressionWithDesc() throws Exception {
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
index 4235406..fc276c8 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
@@ -26,6 +26,7 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
@@ -64,22 +65,39 @@ public class InListExpression extends BaseSingleExpression {
     private boolean hashCodeSet = false;
 
     public static Expression create (List<Expression> children, boolean isNegate, ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
+        if (children.size() == 1) {
+            throw new SQLException("No element in the IN list");
+        }
+
         Expression firstChild = children.get(0);
 
         if (firstChild.isStateless() && (!firstChild.evaluate(null, ptr) || ptr.getLength() == 0)) {
             return LiteralExpression.newConstant(null, PBoolean.INSTANCE, firstChild.getDeterminism());
         }
-        if (children.size() == 2) {
-            return ComparisonExpression.create(isNegate ? CompareOp.NOT_EQUAL : CompareOp.EQUAL, children, ptr, rowKeyOrderOptimizable);
+
+        List<Expression> childrenWithoutNulls = Lists.newArrayList();
+        for (Expression child : children){
+            if(!child.equals(LiteralExpression.newConstant(null))){
+                childrenWithoutNulls.add(child);
+            }
+        }
+        if (childrenWithoutNulls.size() <= 1 ) {
+            // In case of after removing nulls there is no remaining element in the IN list
+            return LiteralExpression.newConstant(false);
         }
+        boolean nullInList = children.size() != childrenWithoutNulls.size();
 
-        boolean addedNull = false;
+        if (childrenWithoutNulls.size() == 2 && !nullInList) {
+            return ComparisonExpression.create(isNegate ? CompareOp.NOT_EQUAL : CompareOp.EQUAL,
+                    childrenWithoutNulls, ptr, rowKeyOrderOptimizable);
+        }
         SQLException sqlE = null;
-        List<Expression> coercedKeyExpressions = Lists.newArrayListWithExpectedSize(children.size());
+        List<Expression> coercedKeyExpressions = Lists.newArrayListWithExpectedSize(childrenWithoutNulls.size());
         coercedKeyExpressions.add(firstChild);
-        for (int i = 1; i < children.size(); i++) {
+        for (int i = 1; i < childrenWithoutNulls.size(); i++) {
             try {
-                Expression rhs = BaseExpression.coerce(firstChild, children.get(i), CompareOp.EQUAL, rowKeyOrderOptimizable);
+                Expression rhs = BaseExpression.coerce(firstChild, childrenWithoutNulls.get(i),
+                        CompareOp.EQUAL, rowKeyOrderOptimizable);
                 coercedKeyExpressions.add(rhs);
             } catch (SQLException e) {
                 // Type mismatch exception or invalid data exception.
@@ -89,11 +107,17 @@ public class InListExpression extends BaseSingleExpression {
                 sqlE = e;
             }
         }
-        if (coercedKeyExpressions.size() == 1) {
-            throw sqlE != null ? sqlE : new SQLException("Only one element in IN list");
+        if (coercedKeyExpressions.size() <= 1 ) {
+            if(nullInList || sqlE == null){
+                // In case of after removing nulls there is no remaining element in the IN list
+                return LiteralExpression.newConstant(false);
+            } else {
+                throw sqlE;
+            }
         }
-        if (coercedKeyExpressions.size() == 2 && addedNull) {
-            return LiteralExpression.newConstant(null, PBoolean.INSTANCE, Determinism.ALWAYS);
+        if (coercedKeyExpressions.size() == 2) {
+            return ComparisonExpression.create(isNegate ? CompareOp.NOT_EQUAL : CompareOp.EQUAL,
+                    coercedKeyExpressions, ptr, rowKeyOrderOptimizable);
         }
         Expression expression = new InListExpression(coercedKeyExpressions, rowKeyOrderOptimizable);
         if (isNegate) {