phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject [1/2] git commit: PHOENIX-127 Query with pk IS NULL OR pk IN (1, 2) return incorrect results (JamesTaylor)
Date Thu, 13 Mar 2014 21:37:11 GMT
Repository: incubator-phoenix
Updated Branches:
  refs/heads/3.0 63f4e0e48 -> 935d652a3


PHOENIX-127 Query with pk IS NULL OR pk IN (1,2) return incorrect results (JamesTaylor)


Project: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/commit/9b034c03
Tree: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/tree/9b034c03
Diff: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/diff/9b034c03

Branch: refs/heads/3.0
Commit: 9b034c03ac5579e2a945a57b40f97eeaa13ade02
Parents: 2d58ded
Author: James Taylor <jamestaylor@apache.org>
Authored: Thu Mar 13 14:10:40 2014 -0700
Committer: James Taylor <jamestaylor@apache.org>
Committed: Thu Mar 13 14:10:40 2014 -0700

----------------------------------------------------------------------
 .../org/apache/phoenix/end2end/IsNullIT.java    | 67 ++++++++++++++++--
 .../org/apache/phoenix/end2end/QueryIT.java     | 74 ++++++++++++++------
 .../end2end/TenantSpecificTablesDDLIT.java      | 39 ++++++-----
 .../phoenix/compile/ExpressionCompiler.java     | 18 ++---
 .../expression/ComparisonExpression.java        |  6 ++
 .../java/org/apache/phoenix/query/KeyRange.java | 16 ++++-
 .../org/apache/phoenix/util/ScanUtilTest.java   | 10 +--
 7 files changed, 174 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
index d9d8364..956e6de 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
@@ -18,14 +18,18 @@
 package org.apache.phoenix.end2end;
 
 import static org.apache.phoenix.util.TestUtil.PHOENIX_JDBC_URL;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
-import java.sql.*;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
 import java.util.Properties;
 
-import org.junit.Test;
-
 import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.Test;
 
 
 public class IsNullIT extends BaseClientManagedTimeIT {
@@ -62,4 +66,59 @@ public class IsNullIT extends BaseClientManagedTimeIT {
         assertEquals(2,rs.getInt(1));
         assertFalse(rs.next());
     }
+    
+    @Test
+    public void testIsNullAsSingleKey() throws Exception {
+        long ts = nextTimestamp();
+        Properties props = new Properties();
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 10));
+        Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        conn.createStatement().execute("CREATE TABLE T(k VARCHAR PRIMARY KEY)");
+        conn.close();
+        
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 20));
+        conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        conn.createStatement().execute("UPSERT INTO T VALUES (null)");
+        conn.createStatement().execute("UPSERT INTO T VALUES ('a')");
+        conn.commit();
+        conn.close();
+        
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 30));
+        conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        ResultSet rs = conn.createStatement().executeQuery("SELECT count(*) FROM T");
+        assertTrue(rs.next());
+        assertEquals(2,rs.getInt(1));
+        rs = conn.createStatement().executeQuery("SELECT count(*) FROM T WHERE k = 'a' or
k is null");
+        assertTrue(rs.next());
+        assertEquals(2,rs.getInt(1));
+        conn.close();
+    }
+    
+    @Test
+    public void testIsNullInCompositeKey() throws Exception {
+        long ts = nextTimestamp();
+        Properties props = new Properties();
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 10));
+        Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        conn.createStatement().execute("CREATE TABLE T(k1 VARCHAR, k2 VARCHAR, CONSTRAINT
pk PRIMARY KEY (k1,k2))");
+        conn.close();
+        
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 20));
+        conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        conn.createStatement().execute("UPSERT INTO T VALUES (null,'a')");
+        conn.createStatement().execute("UPSERT INTO T VALUES ('a','a')");
+        conn.commit();
+        conn.close();
+        
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 30));
+        conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        ResultSet rs = conn.createStatement().executeQuery("SELECT count(*) FROM T");
+        assertTrue(rs.next());
+        assertEquals(2,rs.getInt(1));
+        rs = conn.createStatement().executeQuery("SELECT count(*) FROM T WHERE k1 = 'a' or
k1 is null");
+        assertTrue(rs.next());
+        assertEquals(2,rs.getInt(1));
+        conn.close();
+    }
+    
 }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryIT.java
index 196b644..ec1ed39 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryIT.java
@@ -158,19 +158,30 @@ public class QueryIT extends BaseClientManagedTimeIT {
         }
         assertValuesEqualsResultSet(rs, nestedExpectedResults); 
     }
-    
+
+    /**
+     * Asserts that we find the expected values in the result set. We don't know the order,
since we don't always
+     * have an order by and we're going through indexes, but we assert that each expected
result occurs once as
+     * expected (in any order).
+     */
     private void assertValuesEqualsResultSet(ResultSet rs, List<List<Object>>
expectedResults) throws SQLException {
-        Set<List<Object>> expectedResultsSet = Sets.newHashSet(expectedResults);
+        int expectedCount = expectedResults.size();
         int count = 0;
-        while (rs.next()) {
-            List<Object> results = Lists.newArrayList();
+        List<List<Object>> actualResults = Lists.newArrayList();
+        List<Object> errorResult = null;
+        while (rs.next() && errorResult == null) {
+            List<Object> result = Lists.newArrayList();
             for (int i = 0; i < rs.getMetaData().getColumnCount(); i++) {
-                results.add(rs.getObject(i+1));
+                result.add(rs.getObject(i+1));
             }
-            assertTrue("Expected to find " + results + " in results: " + expectedResults,
expectedResultsSet.contains(results));
+            if (!expectedResults.contains(result)) {
+                errorResult = result;
+            }
+            actualResults.add(result);
             count++;
         }
-        assertEquals(count, expectedResults.size());
+        assertTrue("Could not find " + errorResult + " in expected results: " + expectedResults
+ " with actual results: " + actualResults, errorResult == null);
+        assertEquals(count, expectedCount);
     }
     
     private void assertOneOfValuesEqualsResultSet(ResultSet rs, List<List<Object>>...
expectedResultsArray) throws SQLException {
@@ -571,26 +582,33 @@ public class QueryIT extends BaseClientManagedTimeIT {
         }
     }
     
+    @SuppressWarnings("unchecked")
     @Test
     public void testGroupByCondition() throws Exception {
-        // FIXME: with filter of AND a_integer IN (5,6) was getting 9 rows back
-        // FIXME: with filter of AND (a_integer IN (5,6) OR a_integer IS NULL) was getting
5 rows back
-        String query = "SELECT count(*) FROM aTable WHERE organization_id=? GROUP BY a_integer=6";
         Properties props = new Properties(TEST_PROPERTIES);
-        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 2)); // Execute
at timestamp 2
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 20));
         Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        PreparedStatement statement = conn.prepareStatement("SELECT count(*) FROM aTable
WHERE organization_id=? GROUP BY a_integer=6");
+        statement.setString(1, tenantId);
+        ResultSet rs = statement.executeQuery();
+        assertValueEqualsResultSet(rs, Arrays.<Object>asList(1L,8L));
         try {
-            PreparedStatement statement = conn.prepareStatement(query);
+            statement = conn.prepareStatement("SELECT count(*),a_integer=6 FROM aTable WHERE
organization_id=? and (a_integer IN (5,6) or a_integer is null) GROUP BY a_integer=6");
             statement.setString(1, tenantId);
-            ResultSet rs = statement.executeQuery();
-            assertValueEqualsResultSet(rs, Arrays.<Object>asList(1L,8L));
+            rs = statement.executeQuery();
+            List<List<Object>> expectedResults = Lists.newArrayList(
+                    Arrays.<Object>asList(1L,false),
+                    Arrays.<Object>asList(1L,true));
+            assertValuesEqualsResultSet(rs, expectedResults);
         } finally {
             conn.close();
         }
-        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 4));
+
+        
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 40));
         conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
         try {
-            PreparedStatement statement = conn.prepareStatement("UPSERT into aTable(organization_id,entity_id,a_integer)
values(?,?,null)");
+            statement = conn.prepareStatement("UPSERT into aTable(organization_id,entity_id,a_integer)
values(?,?,null)");
             statement.setString(1, tenantId);
             statement.setString(2, ROW3);
             statement.executeUpdate();
@@ -598,13 +616,29 @@ public class QueryIT extends BaseClientManagedTimeIT {
         } finally {
             conn.close();
         }
-        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 6));
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 60));
         conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
+        statement = conn.prepareStatement("SELECT count(*) FROM aTable WHERE organization_id=?
GROUP BY a_integer=6");
+        statement.setString(1, tenantId);
+        rs = statement.executeQuery();
+        assertValueEqualsResultSet(rs, Arrays.<Object>asList(1L,1L,7L));
+        statement = conn.prepareStatement("SELECT a_integer, entity_id FROM aTable WHERE
organization_id=? and (a_integer IN (5,6) or a_integer is null)");
+        statement.setString(1, tenantId);
+        rs = statement.executeQuery();
+        List<List<Object>> expectedResults = Lists.newArrayList(
+                Arrays.<Object>asList(null,ROW3),
+                Arrays.<Object>asList(5,ROW5),
+                Arrays.<Object>asList(6,ROW6));
+        assertValuesEqualsResultSet(rs, expectedResults);
         try {
-            PreparedStatement statement = conn.prepareStatement(query);
+            statement = conn.prepareStatement("SELECT count(*),a_integer=6 FROM aTable WHERE
organization_id=? and (a_integer IN (5,6) or a_integer is null) GROUP BY a_integer=6");
             statement.setString(1, tenantId);
-            ResultSet rs = statement.executeQuery();
-            assertValueEqualsResultSet(rs, Arrays.<Object>asList(1L,1L,7L));
+            rs = statement.executeQuery();
+            expectedResults = Lists.newArrayList(
+                    Arrays.<Object>asList(1L,null),
+                    Arrays.<Object>asList(1L,false),
+                    Arrays.<Object>asList(1L,true));
+            assertValuesEqualsResultSet(rs, expectedResults);
         } finally {
             conn.close();
         }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java
index 465e266..e77e314 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java
@@ -347,9 +347,10 @@ public class TenantSpecificTablesDDLIT extends BaseTenantSpecificTablesIT
{
     @Test
     public void testTableMetadataScan() throws Exception {
         // create a tenant table with same name for a different tenant to make sure we are
not picking it up in metadata scans for TENANT_ID
-        String secondTenantId = "tenant2";
-        String secondTenatConnectionURL = PHOENIX_JDBC_TENANT_SPECIFIC_URL.replace(TENANT_ID,
 secondTenantId);
-        createTestTable(secondTenatConnectionURL, TENANT_TABLE_DDL, null, nextTimestamp(),
false);
+        String tenantId2 = "tenant2";
+        String secondTenatConnectionURL = PHOENIX_JDBC_TENANT_SPECIFIC_URL.replace(TENANT_ID,
 tenantId2);
+        String tenantTable2 = TENANT_TABLE_NAME+"2";
+        createTestTable(secondTenatConnectionURL, TENANT_TABLE_DDL.replace(TENANT_TABLE_NAME,
tenantTable2), null, nextTimestamp(), false);
         
         Properties props = new Properties();
         props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp()));
@@ -373,17 +374,22 @@ public class TenantSpecificTablesDDLIT extends BaseTenantSpecificTablesIT
{
             rs = meta.getColumns("", null, null, null);
             while (rs.next()) {
                 assertNotEquals(TENANT_TABLE_NAME, rs.getString("TABLE_NAME"));
+                assertNotEquals(tenantTable2, rs.getString("TABLE_NAME"));
             }
             
             // null catalog means across all tenant_ids
-            rs = meta.getSuperTables(null, null, StringUtil.escapeLike(TENANT_TABLE_NAME));
+            rs = meta.getSuperTables(null, null, StringUtil.escapeLike(TENANT_TABLE_NAME)
+ "%");
             assertTrue(rs.next());
             assertEquals(TENANT_ID, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
             assertEquals(TENANT_TABLE_NAME, rs.getString(PhoenixDatabaseMetaData.TABLE_NAME));
             assertEquals(PARENT_TABLE_NAME, rs.getString(PhoenixDatabaseMetaData.SUPERTABLE_NAME));
             assertTrue(rs.next());
-            assertEquals(secondTenantId, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
-            assertEquals(TENANT_TABLE_NAME, rs.getString(PhoenixDatabaseMetaData.TABLE_NAME));
+            assertEquals(TENANT_ID, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
+            assertEquals(TENANT_TABLE_NAME_NO_TENANT_TYPE_ID, rs.getString(PhoenixDatabaseMetaData.TABLE_NAME));
+            assertEquals(PARENT_TABLE_NAME_NO_TENANT_TYPE_ID, rs.getString(PhoenixDatabaseMetaData.SUPERTABLE_NAME));
+            assertTrue(rs.next());
+            assertEquals(tenantId2, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
+            assertEquals(tenantTable2, rs.getString(PhoenixDatabaseMetaData.TABLE_NAME));
             assertEquals(PARENT_TABLE_NAME, rs.getString(PhoenixDatabaseMetaData.SUPERTABLE_NAME));
             assertFalse(rs.next());
             conn.close();
@@ -405,34 +411,35 @@ public class TenantSpecificTablesDDLIT extends BaseTenantSpecificTablesIT
{
             assertTrue(rs.next());
             assertEquals(TENANT_ID, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
             assertTrue(rs.next());
-            assertEquals(secondTenantId, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
+            assertEquals(tenantId2, rs.getString(PhoenixDatabaseMetaData.TABLE_CAT));
             assertFalse(rs.next());
         } finally {
             props.clear();
-            props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp()));
-            Connection secondTenantCon = DriverManager.getConnection(secondTenatConnectionURL,
props);
-            try {
-                secondTenantCon.createStatement().executeUpdate("drop view " + TENANT_TABLE_NAME);
-            } finally {
-                try {secondTenantCon.close();} catch (Exception ignored) {}
-            }
             conn.close();
         }
         
         props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp()));
         conn = DriverManager.getConnection(PHOENIX_JDBC_TENANT_SPECIFIC_URL, props);
         try {   
-            // make sure tenant-specific connections only see their own tables
+            // make sure tenant-specific connections only see their own tables and the global
tables
             DatabaseMetaData meta = conn.getMetaData();
             ResultSet rs = meta.getTables(null, null, null, null);
             assertTrue(rs.next());
+            assertTableMetaData(rs, PhoenixDatabaseMetaData.SYSTEM_CATALOG_SCHEMA, PhoenixDatabaseMetaData.SYSTEM_CATALOG_TABLE,
PTableType.SYSTEM);
+            assertTrue(rs.next());
+            assertTableMetaData(rs, PhoenixDatabaseMetaData.SYSTEM_CATALOG_SCHEMA, PhoenixDatabaseMetaData.TYPE_SEQUENCE,
PTableType.SYSTEM);
+            assertTrue(rs.next());
+            assertTableMetaData(rs, null, PARENT_TABLE_NAME, PTableType.TABLE);
+            assertTrue(rs.next());
+            assertTableMetaData(rs, null, PARENT_TABLE_NAME_NO_TENANT_TYPE_ID, PTableType.TABLE);
+            assertTrue(rs.next());
             assertTableMetaData(rs, null, TENANT_TABLE_NAME, PTableType.VIEW);
             assertTrue(rs.next());
             assertTableMetaData(rs, null, TENANT_TABLE_NAME_NO_TENANT_TYPE_ID, PTableType.VIEW);
             assertFalse(rs.next());
             
             // make sure tenants see parent table's columns and their own
-            rs = meta.getColumns(null, null, null, null);
+            rs = meta.getColumns(null, null, StringUtil.escapeLike(TENANT_TABLE_NAME) + "%",
null);
             assertTrue(rs.next());
             assertColumnMetaData(rs, null, TENANT_TABLE_NAME, "user");
             assertTrue(rs.next());

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/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 0c929c2..c14e6a3 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
@@ -176,7 +176,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
         } else {
             addBindParamMetaData(lhsNode, rhsNode, lhsExpr, rhsExpr);
         }
-        return ComparisonExpression.create(op, children, context.getTempPtr());
+        return wrapGroupByExpression(ComparisonExpression.create(op, children, context.getTempPtr()));
     }
 
     @Override
@@ -186,7 +186,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
 
     @Override
     public Expression visitLeave(AndParseNode node, List<Expression> children) throws
SQLException {
-        return AndExpression.create(children);
+        return wrapGroupByExpression(AndExpression.create(children));
     }
 
     @Override
@@ -221,7 +221,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
 
     @Override
     public Expression visitLeave(OrParseNode node, List<Expression> children) throws
SQLException {
-        return orExpression(children);
+        return wrapGroupByExpression(orExpression(children));
     }
 
     @Override
@@ -464,7 +464,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
         if (node.isNegate()) {
             expression = new NotExpression(expression);
         }
-        return expression;
+        return wrapGroupByExpression(expression);
     }
 
 
@@ -483,7 +483,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
         if (childNode instanceof BindParseNode) { // TODO: valid/possibe?
             context.getBindManager().addParamMetaData((BindParseNode)childNode, child);
         }
-        return NotExpression.create(child, context.getTempPtr());
+        return wrapGroupByExpression(NotExpression.create(child, context.getTempPtr()));
     }
 
     @Override
@@ -513,7 +513,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
                 expr =  CastParseNode.convertToRoundExpressionIfNeeded(fromDataType, targetDataType,
children);
             }
         }
-        return CoerceExpression.create(expr, targetDataType); 
+        return wrapGroupByExpression(CoerceExpression.create(expr, targetDataType)); 
     }
     
    @Override
@@ -542,7 +542,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
                 context.getBindManager().addParamMetaData((BindParseNode)childNode, firstChild);
             }
         }
-        return InListExpression.create(inChildren, node.isNegate(), ptr);
+        return wrapGroupByExpression(InListExpression.create(inChildren, node.isNegate(),
ptr));
     }
 
     private static final PDatum DECIMAL_DATUM = new PDatum() {
@@ -606,7 +606,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
         if (childNode instanceof BindParseNode) { // TODO: valid/possibe?
             context.getBindManager().addParamMetaData((BindParseNode)childNode, child);
         }
-        return IsNullExpression.create(child, node.isNegate(), context.getTempPtr());
+        return wrapGroupByExpression(IsNullExpression.create(child, node.isNegate(), context.getTempPtr()));
     }
 
     private static interface ArithmeticExpressionFactory {
@@ -1068,7 +1068,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
     public Expression visitLeave(RowValueConstructorParseNode node, List<Expression>
l) throws SQLException {
         // Don't trim trailing nulls here, as we'd potentially be dropping bind
         // variables that aren't bound yet.
-        return new RowValueConstructorExpression(l, node.isStateless());
+        return wrapGroupByExpression(new RowValueConstructorExpression(l, node.isStateless()));
     }
 
 	@Override

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/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 6256b6f..be32b55 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
@@ -297,6 +297,9 @@ public class ComparisonExpression extends BaseCompoundExpression {
         if (!children.get(0).evaluate(tuple, ptr)) {
             return false;
         }
+        if (ptr.getLength() == 0) { // null comparison evals to null
+            return true;
+        }
         byte[] lhsBytes = ptr.get();
         int lhsOffset = ptr.getOffset();
         int lhsLength = ptr.getLength();
@@ -306,6 +309,9 @@ public class ComparisonExpression extends BaseCompoundExpression {
         if (!children.get(1).evaluate(tuple, ptr)) {
             return false;
         }
+        if (ptr.getLength() == 0) { // null comparison evals to null
+            return true;
+        }
         
         byte[] rhsBytes = ptr.get();
         int rhsOffset = ptr.getOffset();

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
index 0caa302..04c88bc 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
@@ -32,12 +32,12 @@ import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.io.Writable;
 import org.apache.hadoop.io.WritableUtils;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.util.ByteUtil;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.Lists;
-import org.apache.phoenix.schema.SortOrder;
-import org.apache.phoenix.util.ByteUtil;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 
@@ -123,6 +123,10 @@ public class KeyRange implements Writable {
         if (lowerRange == null || upperRange == null) {
             return EMPTY_RANGE;
         }
+        // Need to treat null differently for a point range
+        if (lowerRange.length == 0 && upperRange.length == 0 && lowerInclusive
&& upperInclusive) {
+            return IS_NULL_RANGE;
+        }
         boolean unboundLower = false;
         boolean unboundUpper = false;
         if (lowerRange.length == 0) {
@@ -321,6 +325,14 @@ public class KeyRange implements Writable {
         byte[] newUpperRange;
         boolean newLowerInclusive;
         boolean newUpperInclusive;
+        // Special case for null, is it is never included another range
+        // except for null itself.
+        if (this == IS_NULL_RANGE) {
+            if (range == IS_NULL_RANGE) {
+                return IS_NULL_RANGE;
+            }
+            return EMPTY_RANGE;
+        }
         if (lowerUnbound()) {
             newLowerRange = range.lowerRange;
             newLowerInclusive = range.lowerInclusive;

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/9b034c03/phoenix-core/src/test/java/org/apache/phoenix/util/ScanUtilTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/ScanUtilTest.java b/phoenix-core/src/test/java/org/apache/phoenix/util/ScanUtilTest.java
index 868dd46..58f136c 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/util/ScanUtilTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/util/ScanUtilTest.java
@@ -185,7 +185,7 @@ public class ScanUtilTest {
         testCases.addAll(
             foreach(new KeyRange[][]{{
                     PDataType.CHAR.getKeyRange(Bytes.toBytes("a"), true, Bytes.toBytes("a"),
true),},{
-                    PDataType.CHAR.getKeyRange(KeyRange.UNBOUND, true, KeyRange.UNBOUND,
true),},{
+                        KeyRange.EVERYTHING_RANGE,},{
                     PDataType.CHAR.getKeyRange(Bytes.toBytes("A"), false, Bytes.toBytes("B"),
true),}},
                 new int[] {1,1,1},
                 ByteUtil.fillKey(PDataType.VARCHAR.toBytes("a"), 1),
@@ -195,7 +195,7 @@ public class ScanUtilTest {
         testCases.addAll(
                 foreach(new KeyRange[][]{{
                         PDataType.CHAR.getKeyRange(Bytes.toBytes("a"), true, Bytes.toBytes("a"),
true),},{
-                        PDataType.CHAR.getKeyRange(KeyRange.UNBOUND, true, KeyRange.UNBOUND,
true),}},
+                            KeyRange.EVERYTHING_RANGE,}},
                     new int[] {1,1},
                     ByteUtil.concat(PDataType.VARCHAR.toBytes("a")),
                     Bound.LOWER
@@ -204,7 +204,7 @@ public class ScanUtilTest {
         testCases.addAll(
             foreach(new KeyRange[][]{{
                     PDataType.CHAR.getKeyRange(Bytes.toBytes("a"), true, Bytes.toBytes("a"),
true),},{
-                    PDataType.CHAR.getKeyRange(KeyRange.UNBOUND, true, KeyRange.UNBOUND,
true),},{
+                        KeyRange.EVERYTHING_RANGE,},{
                     PDataType.CHAR.getKeyRange(Bytes.toBytes("A"), true, Bytes.toBytes("B"),
true),}},
                 new int[] {1,1,1},
                 ByteUtil.concat(PDataType.VARCHAR.toBytes("a")),
@@ -280,7 +280,7 @@ public class ScanUtilTest {
         testCases.addAll(
             foreach(new KeyRange[][]{{
                     PDataType.CHAR.getKeyRange(Bytes.toBytes("a"), true, Bytes.toBytes("a"),
true),},{
-                    PDataType.CHAR.getKeyRange(KeyRange.UNBOUND, true, KeyRange.UNBOUND,
true),}},
+                        KeyRange.EVERYTHING_RANGE,}},
                 new int[] {1,1},
                 ByteUtil.fillKey(PDataType.VARCHAR.toBytes("b"), 1),
                 Bound.UPPER
@@ -289,7 +289,7 @@ public class ScanUtilTest {
         testCases.addAll(
             foreach(new KeyRange[][]{{
                     PDataType.CHAR.getKeyRange(Bytes.toBytes("a"), true, Bytes.toBytes("a"),
true),},{
-                    PDataType.CHAR.getKeyRange(KeyRange.UNBOUND, true, KeyRange.UNBOUND,
true),}},
+                    KeyRange.EVERYTHING_RANGE,}},
                 new int[] {1,1},
                 ByteUtil.concat(PDataType.VARCHAR.toBytes("b")),
                 Bound.UPPER


Mime
View raw message