phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject [1/2] phoenix git commit: PHOENIX-2691 Exception while unpacking resultset containing VARCHAR ARRAY of unspecified length
Date Fri, 19 Feb 2016 06:29:10 GMT
Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-1.0 699362488 -> aa362e744


PHOENIX-2691 Exception while unpacking resultset containing VARCHAR ARRAY of unspecified length


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

Branch: refs/heads/4.x-HBase-1.0
Commit: 264c8700595ef440d6de11933386df8e42e8bd4b
Parents: 6993624
Author: James Taylor <jtaylor@salesforce.com>
Authored: Thu Feb 18 14:59:29 2016 -0800
Committer: James Taylor <jtaylor@salesforce.com>
Committed: Thu Feb 18 22:28:06 2016 -0800

----------------------------------------------------------------------
 .../apache/phoenix/end2end/GroupByCaseIT.java   | 35 +++++++++
 .../apache/phoenix/compile/GroupByCompiler.java | 74 ++++++++++++--------
 .../phoenix/exception/SQLExceptionCode.java     |  5 +-
 .../java/org/apache/phoenix/util/IndexUtil.java |  6 +-
 .../phoenix/compile/QueryCompilerTest.java      | 36 ++++++++++
 5 files changed, 122 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/264c8700/phoenix-core/src/it/java/org/apache/phoenix/end2end/GroupByCaseIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/GroupByCaseIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/GroupByCaseIT.java
index 0f1568c..172f9f7 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/GroupByCaseIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/GroupByCaseIT.java
@@ -226,4 +226,39 @@ public class GroupByCaseIT extends BaseHBaseManagedTimeIT {
         conn.close();
     }
 
+
+    @Test
+    public void testGroupByArray() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        conn.createStatement().execute("CREATE TABLE test1(\n" + 
+                "  a VARCHAR NOT NULL,\n" + 
+                "  b VARCHAR,\n" + 
+                "  c INTEGER,\n" + 
+                "  d VARCHAR,\n" + 
+                "  e VARCHAR ARRAY,\n" + 
+                "  f BIGINT,\n" + 
+                "  g BIGINT,\n" + 
+                "  CONSTRAINT pk PRIMARY KEY(a)\n" + 
+                ")");
+        conn.createStatement().execute("UPSERT INTO test1 VALUES('1', 'val', 100, 'a', ARRAY
['b'], 1, 2)");
+        conn.createStatement().execute("UPSERT INTO test1 VALUES('2', 'val', 100, 'a', ARRAY
['b'], 3, 4)");
+        conn.createStatement().execute("UPSERT INTO test1 VALUES('3', 'val', 100, 'a', ARRAY
['b','c'], 5, 6)");
+        conn.commit();
+        
+        ResultSet rs = conn.createStatement().executeQuery("SELECT c, SUM(f + g) AS sumone,
d, e\n" + 
+                "FROM test1\n" + 
+                "WHERE b = 'val'\n" + 
+                "  AND a IN ('1','2','3')\n" + 
+                "GROUP BY c, d, e\n" + 
+                "ORDER BY sumone DESC");
+        assertTrue(rs.next());
+        assertEquals(100, rs.getInt(1));
+        assertEquals(11, rs.getLong(2));
+        assertTrue(rs.next());
+        assertEquals(100, rs.getInt(1));
+        assertEquals(10, rs.getLong(2));
+        assertFalse(rs.next());
+        conn.close();
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/264c8700/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
index 7d9df02..85478bf 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
@@ -38,8 +38,8 @@ import org.apache.phoenix.parse.SelectStatement;
 import org.apache.phoenix.schema.AmbiguousColumnException;
 import org.apache.phoenix.schema.ColumnNotFoundException;
 import org.apache.phoenix.schema.types.PDataType;
-import org.apache.phoenix.schema.types.PDecimal;
-import org.apache.phoenix.schema.types.PVarchar;
+import org.apache.phoenix.schema.types.PVarbinary;
+import org.apache.phoenix.util.IndexUtil;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
@@ -217,34 +217,53 @@ public class GroupByCompiler {
                 public int compare(Pair<Integer,Expression> gb1, Pair<Integer,Expression>
gb2) {
                     Expression e1 = gb1.getSecond();
                     Expression e2 = gb2.getSecond();
-                    boolean isFixed1 = e1.getDataType().isFixedWidth();
-                    boolean isFixed2 = e2.getDataType().isFixedWidth();
+                    PDataType t1 = e1.getDataType();
+                    PDataType t2 = e2.getDataType();
+                    boolean isFixed1 = t1.isFixedWidth();
+                    boolean isFixed2 = t2.isFixedWidth();
                     boolean isFixedNullable1 = e1.isNullable() &&isFixed1;
                     boolean isFixedNullable2 = e2.isNullable() && isFixed2;
-                    if (isFixedNullable1 == isFixedNullable2) {
-                        if (isFixed1 == isFixed2) {
-                            // Not strictly necessary, but forces the order to match the
schema
-                            // column order (with PK columns before value columns).
-                            //return o1.getColumnPosition() - o2.getColumnPosition();
-                            return gb1.getFirst() - gb2.getFirst();
-                        } else if (isFixed1) {
-                            return -1;
-                        } else {
+                    boolean oae1 = onlyAtEndType(e1);
+                    boolean oae2 = onlyAtEndType(e2);
+                    if (oae1 == oae2) {
+                        if (isFixedNullable1 == isFixedNullable2) {
+                            if (isFixed1 == isFixed2) {
+                                // Not strictly necessary, but forces the order to match
the schema
+                                // column order (with PK columns before value columns).
+                                //return o1.getColumnPosition() - o2.getColumnPosition();
+                                return gb1.getFirst() - gb2.getFirst();
+                            } else if (isFixed1) {
+                                return -1;
+                            } else {
+                                return 1;
+                            }
+                        } else if (isFixedNullable1) {
                             return 1;
+                        } else {
+                            return -1;
                         }
-                    } else if (isFixedNullable1) {
+                    } else if (oae1) {
                         return 1;
                     } else {
                         return -1;
                     }
                 }
             });
+            boolean foundOnlyAtEndType = false;
             for (Pair<Integer,Expression> groupBy : groupBys) {
-                expressions.add(groupBy.getSecond());
+                Expression e = groupBy.getSecond();
+                if (onlyAtEndType(e)) {
+                    if (foundOnlyAtEndType) {
+                        throw new SQLExceptionInfo.Builder(SQLExceptionCode.UNSUPPORTED_GROUP_BY_EXPRESSIONS)
+                        .setMessage(e.toString()).build().buildException();
+                    }
+                    foundOnlyAtEndType  = true;
+                }
+                expressions.add(e);
             }
             for (int i = expressions.size()-2; i >= 0; i--) {
                 Expression expression = expressions.get(i);
-                PDataType keyType = getKeyType(expression);
+                PDataType keyType = getGroupByDataType(expression);
                 if (keyType == expression.getDataType()) {
                     continue;
                 }
@@ -263,19 +282,16 @@ public class GroupByCompiler {
         return groupBy;
     }
     
-    private static PDataType getKeyType(Expression expression) {
-        PDataType type = expression.getDataType();
-        if (!expression.isNullable() || !type.isFixedWidth()) {
-            return type;
-        }
-        if (type.isCastableTo(PDecimal.INSTANCE)) {
-            return PDecimal.INSTANCE;
-        }
-        if (type.isCastableTo(PVarchar.INSTANCE)) {
-            return PVarchar.INSTANCE;
-        }
-        // This might happen if someone tries to group by an array
-        throw new IllegalStateException("Multiple occurrences of type " + type + " may not
occur in a GROUP BY clause");
+    private static boolean onlyAtEndType(Expression expression) {
+        // Due to the encoding schema of these types, they may only be
+        // used once in a group by and are located at the end of the
+        // group by row key.
+        PDataType type = getGroupByDataType(expression);
+        return type.isArrayType() || type == PVarbinary.INSTANCE;
+    }
+    
+    private static PDataType getGroupByDataType(Expression expression) {
+        return IndexUtil.getIndexColumnDataType(expression.isNullable(), expression.getDataType());
     }
     
     private GroupByCompiler() {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/264c8700/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
b/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
index 7ddd14c..65cb6db 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
@@ -229,7 +229,6 @@ public enum SQLExceptionCode {
     AGGREGATE_WITH_NOT_GROUP_BY_COLUMN(1018, "42Y27", "Aggregate may not contain columns
not in GROUP BY."),
     ONLY_AGGREGATE_IN_HAVING_CLAUSE(1019, "42Y26", "Only aggregate maybe used in the HAVING
clause."),
     UPSERT_COLUMN_NUMBERS_MISMATCH(1020, "42Y60", "Number of columns upserting must match
number of values."),
-    NO_TABLE_SPECIFIED_FOR_WILDCARD_SELECT(1057, "42Y10", "No table specified for wildcard
select."),
     // Table properties exception.
     INVALID_BUCKET_NUM(1021, "42Y80", "Salt bucket numbers should be with 1 and 256."),
     NO_SPLITS_ON_SALTED_TABLE(1022, "42Y81", "Should not specify split points on salted table
with default row key order."),
@@ -263,8 +262,10 @@ public enum SQLExceptionCode {
     UNALLOWED_LOCAL_INDEXES(1055, "43A12", "Local secondary indexes are configured to not
be allowed."),
     
     DESC_VARBINARY_NOT_SUPPORTED(1056, "43A13", "Descending VARBINARY columns not supported."),
+    NO_TABLE_SPECIFIED_FOR_WILDCARD_SELECT(1057, "42Y10", "No table specified for wildcard
select."),
+    UNSUPPORTED_GROUP_BY_EXPRESSIONS(1058, "43A14", "Only a single VARBINARY, ARRAY, or nullable
BINARY type may be referenced in a GROUP BY."),
     
-    DEFAULT_COLUMN_FAMILY_ON_SHARED_TABLE(1069, "43A13", "Default column family not allowed
on VIEW or shared INDEX."),
+    DEFAULT_COLUMN_FAMILY_ON_SHARED_TABLE(1069, "43A69", "Default column family not allowed
on VIEW or shared INDEX."),
     ONLY_TABLE_MAY_BE_DECLARED_TRANSACTIONAL(1070, "44A01", "Only tables may be declared
as transactional."),
     TX_MAY_NOT_SWITCH_TO_NON_TX(1071, "44A02", "A transactional table may not be switched
to non transactional."),
 	STORE_NULLS_MUST_BE_TRUE_FOR_TRANSACTIONAL(1072, "44A03", "Store nulls must be true when
a table is transactional."),

http://git-wip-us.apache.org/repos/asf/phoenix/blob/264c8700/phoenix-core/src/main/java/org/apache/phoenix/util/IndexUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/IndexUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/IndexUtil.java
index ddc2b5c..06542ed 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/IndexUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/IndexUtil.java
@@ -87,10 +87,10 @@ import org.apache.phoenix.schema.types.PDecimal;
 import org.apache.phoenix.schema.types.PVarbinary;
 import org.apache.phoenix.schema.types.PVarchar;
 
-import co.cask.tephra.TxConstants;
-
 import com.google.common.collect.Lists;
 
+import co.cask.tephra.TxConstants;
+
 public class IndexUtil {
     public static final String INDEX_COLUMN_NAME_SEP = ":";
     public static final byte[] INDEX_COLUMN_NAME_SEP_BYTES = Bytes.toBytes(INDEX_COLUMN_NAME_SEP);
@@ -129,7 +129,7 @@ public class IndexUtil {
         if (PBinary.INSTANCE.equals(dataType)) {
             return PVarbinary.INSTANCE;
         }
-        throw new IllegalArgumentException("Unsupported non nullable index type " + dataType);
+        throw new IllegalArgumentException("Unsupported non nullable type " + dataType);
     }
     
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/264c8700/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
index b3baa73..cfec967 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
@@ -2136,6 +2136,42 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest
{
     }
 
     @Test
+    public void testGroupByVarbinaryOrArray() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        conn.createStatement().execute("CREATE TABLE T1 (PK VARCHAR PRIMARY KEY, c1 VARCHAR,
c2 VARBINARY, C3 VARCHAR ARRAY, c4 VARBINARY, C5 VARCHAR ARRAY, C6 BINARY(10)) ");
+        try {
+            conn.createStatement().executeQuery("SELECT c1 FROM t1 GROUP BY c1,c2,c3");
+            fail();
+        } catch(SQLException e) {
+            assertEquals(SQLExceptionCode.UNSUPPORTED_GROUP_BY_EXPRESSIONS.getErrorCode(),
e.getErrorCode());
+        }
+        try {
+            conn.createStatement().executeQuery("SELECT c1 FROM t1 GROUP BY c1,c3,c2");
+            fail();
+        } catch(SQLException e) {
+            assertEquals(SQLExceptionCode.UNSUPPORTED_GROUP_BY_EXPRESSIONS.getErrorCode(),
e.getErrorCode());
+        }
+        try {
+            conn.createStatement().executeQuery("SELECT c1 FROM t1 GROUP BY c1,c2,c4");
+            fail();
+        } catch(SQLException e) {
+            assertEquals(SQLExceptionCode.UNSUPPORTED_GROUP_BY_EXPRESSIONS.getErrorCode(),
e.getErrorCode());
+        }
+        try {
+            conn.createStatement().executeQuery("SELECT c1 FROM t1 GROUP BY c1,c3,c5");
+            fail();
+        } catch(SQLException e) {
+            assertEquals(SQLExceptionCode.UNSUPPORTED_GROUP_BY_EXPRESSIONS.getErrorCode(),
e.getErrorCode());
+        }
+        try {
+            conn.createStatement().executeQuery("SELECT c1 FROM t1 GROUP BY c1,c6,c5");
+            fail();
+        } catch(SQLException e) {
+            assertEquals(SQLExceptionCode.UNSUPPORTED_GROUP_BY_EXPRESSIONS.getErrorCode(),
e.getErrorCode());
+        }
+    }
+    
+    @Test
     public void testQueryWithSCN() throws Exception {
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
         props.put(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(1000));


Mime
View raw message