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/cac03056 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/cac03056 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/cac03056 Branch: refs/heads/calcite Commit: cac03056578170a82ba812aa4648e0e5b1a1bbb6 Parents: 45a9d67 Author: James Taylor <jtay...@salesforce.com> Authored: Thu Feb 18 14:59:29 2016 -0800 Committer: James Taylor <jtay...@salesforce.com> Committed: Thu Feb 18 15:14:48 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/cac03056/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/cac03056/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/cac03056/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/cac03056/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 98b88f4..8fc0c7e 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/cac03056/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));
