This is an automated email from the ASF dual-hosted git repository. stoty pushed a commit to branch 4.x in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push: new a4cbf11 PHOENIX-5065 Inconsistent treatment of NULL and empty string a4cbf11 is described below commit a4cbf11cb6655236756400e68870746ff4482b5a Author: Richard Antal <antal97rich...@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 f74f7d7..0aabdcd 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 4ee08ee..bd86aaa 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) {