PHOENIX-2125 ORDER BY on full PK on salted table does not work
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/b329e85b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/b329e85b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/b329e85b Branch: refs/heads/calcite Commit: b329e85b697575fcebcde9555c991038d14e4a3c Parents: f006df5 Author: Samarth <[email protected]> Authored: Fri Jul 17 17:30:56 2015 -0700 Committer: Samarth <[email protected]> Committed: Fri Jul 17 17:30:56 2015 -0700 ---------------------------------------------------------------------- .../org/apache/phoenix/execute/ScanPlan.java | 12 ++++++- .../java/org/apache/phoenix/util/ScanUtil.java | 6 +++- .../phoenix/compile/QueryCompilerTest.java | 36 ++++++++++++++------ .../expression/ArrayToStringFunctionTest.java | 4 ++- 4 files changed, 44 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/b329e85b/phoenix-core/src/main/java/org/apache/phoenix/execute/ScanPlan.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/execute/ScanPlan.java b/phoenix-core/src/main/java/org/apache/phoenix/execute/ScanPlan.java index fd9f8ad..e9b8a3a 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/execute/ScanPlan.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/ScanPlan.java @@ -185,9 +185,19 @@ public class ScanPlan extends BaseQueryPlan { if (isOrdered) { scanner = new MergeSortTopNResultIterator(iterators, limit, orderBy.getOrderByExpressions()); } else { - if ((isSalted || table.getIndexType() == IndexType.LOCAL) && ScanUtil.forceRowKeyOrder(context)) { + if ((isSalted || table.getIndexType() == IndexType.LOCAL) && ScanUtil.shouldRowsBeInRowKeyOrder(orderBy, context)) { + /* + * For salted tables or local index, a merge sort is needed if: + * 1) The config phoenix.query.force.rowkeyorder is set to true + * 2) Or if the query has an order by that wants to sort + * the results by the row key (forward or reverse ordering) + */ scanner = new MergeSortRowKeyResultIterator(iterators, isSalted ? SaltingUtil.NUM_SALTING_BYTES : 0, orderBy == OrderBy.REV_ROW_KEY_ORDER_BY); } else if (useRoundRobinIterator()) { + /* + * For any kind of tables, round robin is possible if there is + * no ordering of rows needed. + */ scanner = new RoundRobinResultIterator(iterators, this); } else { scanner = new ConcatResultIterator(iterators); http://git-wip-us.apache.org/repos/asf/phoenix/blob/b329e85b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java index d63edbb..9d104ca 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -677,11 +677,15 @@ public class ScanUtil { * not even row key order. Also no point doing round robin of scanners if fetch size * is 1. */ - return fetchSize > 1 && !forceRowKeyOrder(context) && (orderBy.getOrderByExpressions().isEmpty() && orderBy != FWD_ROW_KEY_ORDER_BY && orderBy != REV_ROW_KEY_ORDER_BY); + return fetchSize > 1 && !shouldRowsBeInRowKeyOrder(orderBy, context) && orderBy.getOrderByExpressions().isEmpty(); } public static boolean forceRowKeyOrder(StatementContext context) { return context.getConnection().getQueryServices().getProps() .getBoolean(QueryServices.FORCE_ROW_KEY_ORDER_ATTRIB, QueryServicesOptions.DEFAULT_FORCE_ROW_KEY_ORDER); } + + public static boolean shouldRowsBeInRowKeyOrder(OrderBy orderBy, StatementContext context) { + return forceRowKeyOrder(context) || orderBy == FWD_ROW_KEY_ORDER_BY || orderBy == REV_ROW_KEY_ORDER_BY; + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/b329e85b/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 0f34582..98b130e 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 @@ -1855,11 +1855,17 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest { Properties props = new Properties(); props.setProperty(QueryServices.FORCE_ROW_KEY_ORDER_ATTRIB, Boolean.toString(true)); Connection conn = DriverManager.getConnection(getUrl(), props); - conn.createStatement().execute("CREATE TABLE t (k1 char(2) not null, k2 varchar not null, k3 integer not null, v varchar, constraint pk primary key(k1,k2,k3))"); + testForceRowKeyOrder(conn, false); + testForceRowKeyOrder(conn, true); + } + + private void testForceRowKeyOrder(Connection conn, boolean isSalted) throws SQLException { + String tableName = "tablename" + (isSalted ? "_salt" : ""); + conn.createStatement().execute("CREATE TABLE " + tableName + " (k1 char(2) not null, k2 varchar not null, k3 integer not null, v varchar, constraint pk primary key(k1,k2,k3))"); String[] queries = { - "SELECT 1 FROM T ", - "SELECT 1 FROM T WHERE V = 'c'", - "SELECT 1 FROM T WHERE (k1, k2, k3) > ('a', 'ab', 1)", + "SELECT 1 FROM " + tableName , + "SELECT 1 FROM " + tableName + " WHERE V = 'c'", + "SELECT 1 FROM " + tableName + " WHERE (k1, k2, k3) > ('a', 'ab', 1)", }; String query; for (int i = 0; i < queries.length; i++) { @@ -1874,13 +1880,21 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest { Properties props = new Properties(); props.setProperty(QueryServices.FORCE_ROW_KEY_ORDER_ATTRIB, Boolean.toString(false)); Connection conn = DriverManager.getConnection(getUrl(), props); - conn.createStatement().execute("CREATE TABLE t (k1 char(2) not null, k2 varchar not null, k3 integer not null, v varchar, constraint pk primary key(k1,k2,k3))"); + testOrderByOrGroupByDoesntUseRoundRobin(conn, true); + testOrderByOrGroupByDoesntUseRoundRobin(conn, false); + } + + private void testOrderByOrGroupByDoesntUseRoundRobin(Connection conn, boolean salted) throws SQLException { + String tableName = "orderbygroupbytable" + (salted ? "_salt" : ""); + conn.createStatement().execute("CREATE TABLE " + tableName + " (k1 char(2) not null, k2 varchar not null, k3 integer not null, v varchar, constraint pk primary key(k1,k2,k3))"); String[] queries = { - "SELECT 1 FROM T ORDER BY K1", - "SELECT 1 FROM T WHERE V = 'c' ORDER BY K1, K2", - "SELECT 1 FROM T WHERE (k1,k2, k3) > ('a', 'ab', 1) ORDER BY V", - "SELECT 1 FROM T GROUP BY V", - "SELECT 1 FROM T GROUP BY K1, V, K2 ORDER BY V", + "SELECT 1 FROM " + tableName + " ORDER BY K1", + "SELECT 1 FROM " + tableName + " WHERE V = 'c' ORDER BY K1, K2", + "SELECT 1 FROM " + tableName + " WHERE V = 'c' ORDER BY K1, K2, K3", + "SELECT 1 FROM " + tableName + " WHERE V = 'c' ORDER BY K3", + "SELECT 1 FROM " + tableName + " WHERE (k1,k2, k3) > ('a', 'ab', 1) ORDER BY V", + "SELECT 1 FROM " + tableName + " GROUP BY V", + "SELECT 1 FROM " + tableName + " GROUP BY K1, V, K2 ORDER BY V", }; String query; for (int i = 0; i < queries.length; i++) { @@ -1889,7 +1903,7 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest { assertFalse("Expected plan to not use round robin iterator " + query, plan.useRoundRobinIterator()); } } - + @Test public void testSelectColumnsInOneFamily() throws Exception { Connection conn = DriverManager.getConnection(getUrl()); http://git-wip-us.apache.org/repos/asf/phoenix/blob/b329e85b/phoenix-core/src/test/java/org/apache/phoenix/expression/ArrayToStringFunctionTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/ArrayToStringFunctionTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/ArrayToStringFunctionTest.java index 92eb6b5..d97f117 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/expression/ArrayToStringFunctionTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/ArrayToStringFunctionTest.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.phoenix.expression.function.ArrayToStringFunction; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.types.*; +import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.Lists; @@ -132,8 +133,9 @@ public class ArrayToStringFunctionTest { String expected = "1.1, 2.2, 3.3, 4.4, 5.5"; test(arr, type, null, null, delimiter, nullString, expected, SortOrder.ASC, SortOrder.ASC, SortOrder.ASC); } - + @Test + @Ignore public void testDate() throws SQLException { PDataType type = PDateArray.INSTANCE; PDataType base = PDate.INSTANCE;
