PHOENIX-4690 GroupBy expressions should follow the order of PK Columns if GroupBy is orderPreserving
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/153b357d Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/153b357d Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/153b357d Branch: refs/heads/system-catalog Commit: 153b357d5b9b8663c13e22b42227dffe12d176f3 Parents: 34f93d7 Author: chenglei <cheng...@apache.org> Authored: Wed Apr 18 16:42:31 2018 +0800 Committer: chenglei <cheng...@apache.org> Committed: Wed Apr 18 16:42:31 2018 +0800 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/AggregateIT.java | 107 +++++++++++++++--- .../org/apache/phoenix/end2end/OrderByIT.java | 18 +-- .../apache/phoenix/end2end/join/SubqueryIT.java | 8 +- .../join/SubqueryUsingSortMergeJoinIT.java | 12 +- .../apache/phoenix/compile/GroupByCompiler.java | 19 +++- .../phoenix/compile/OrderPreservingTracker.java | 13 +++ .../phoenix/compile/QueryCompilerTest.java | 110 +++++++++++++++++++ .../java/org/apache/phoenix/util/TestUtil.java | 20 ++++ 8 files changed, 267 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java index b6c5a06..2059311 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.apache.phoenix.util.TestUtil.assertResultSet; import java.io.IOException; import java.sql.Connection; @@ -1016,20 +1017,100 @@ public class AggregateIT extends ParallelStatsDisabledIT { } } - private void assertResultSet(ResultSet rs,Object[][] rows) throws Exception { - for(int rowIndex=0;rowIndex<rows.length;rowIndex++) { - assertTrue(rs.next()); - for(int columnIndex=1;columnIndex<= rows[rowIndex].length;columnIndex++) { - Object realValue=rs.getObject(columnIndex); - Object expectedValue=rows[rowIndex][columnIndex-1]; - if(realValue==null) { - assertTrue(expectedValue==null); - } - else { - assertTrue(realValue.equals(expectedValue)); - } + @Test + public void testGroupByOrderMatchPkColumnOrderBug4690() throws Exception { + this.doTestGroupByOrderMatchPkColumnOrderBug4690(false, false); + this.doTestGroupByOrderMatchPkColumnOrderBug4690(false, true); + this.doTestGroupByOrderMatchPkColumnOrderBug4690(true, false); + this.doTestGroupByOrderMatchPkColumnOrderBug4690(true, true); + } + + private void doTestGroupByOrderMatchPkColumnOrderBug4690(boolean desc ,boolean salted) throws Exception { + Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); + Connection conn = null; + try { + conn = DriverManager.getConnection(getUrl(), props); + String tableName = generateUniqueName(); + String sql = "create table " + tableName + "( "+ + " pk1 integer not null , " + + " pk2 integer not null, " + + " pk3 integer not null," + + " pk4 integer not null,"+ + " v integer, " + + " CONSTRAINT TEST_PK PRIMARY KEY ( "+ + "pk1 "+(desc ? "desc" : "")+", "+ + "pk2 "+(desc ? "desc" : "")+", "+ + "pk3 "+(desc ? "desc" : "")+", "+ + "pk4 "+(desc ? "desc" : "")+ + " )) "+(salted ? "SALT_BUCKETS =4" : "split on(2)"); + conn.createStatement().execute(sql); + + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (1,8,10,20,30)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (1,8,11,21,31)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (1,9,5 ,22,32)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (1,9,6 ,12,33)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (1,9,6 ,13,34)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (1,9,7 ,8,35)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2,3,15,25,35)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2,7,16,26,36)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2,7,17,27,37)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (3,2,18,28,38)"); + conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (3,2,19,29,39)"); + conn.commit(); + + sql = "select pk2,pk1,count(v) from " + tableName + " group by pk2,pk1 order by pk2,pk1"; + ResultSet rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{2,3,2L},{3,2,1L},{7,2,2L},{8,1,2L},{9,1,4L}}); + + sql = "select pk1,pk2,count(v) from " + tableName + " group by pk2,pk1 order by pk1,pk2"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{1,8,2L},{1,9,4L},{2,3,1L},{2,7,2L},{3,2,2L}}); + + sql = "select pk2,pk1,count(v) from " + tableName + " group by pk2,pk1 order by pk2 desc,pk1 desc"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{9,1,4L},{8,1,2L},{7,2,2L},{3,2,1L},{2,3,2L}}); + + sql = "select pk1,pk2,count(v) from " + tableName + " group by pk2,pk1 order by pk1 desc,pk2 desc"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{3,2,2L},{2,7,2L},{2,3,1L},{1,9,4L},{1,8,2L}}); + + + sql = "select pk3,pk2,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk3,pk2"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{5,9,1L},{6,9,2L},{7,9,1L},{10,8,1L},{11,8,1L}}); + + sql = "select pk2,pk3,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk2,pk3"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{8,10,1L},{8,11,1L},{9,5,1L},{9,6,2L},{9,7,1L}}); + + sql = "select pk3,pk2,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk3 desc,pk2 desc"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{11,8,1L},{10,8,1L},{7,9,1L},{6,9,2L},{5,9,1L}}); + + sql = "select pk2,pk3,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk2 desc,pk3 desc"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{9,7,1L},{9,6,2L},{9,5,1L},{8,11,1L},{8,10,1L}}); + + + sql = "select pk4,pk3,pk1,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk4,pk3,pk1"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{8,7,1,1L},{12,6,1,1L},{13,6,1,1L},{22,5,1,1L}}); + + sql = "select pk1,pk3,pk4,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk1,pk3,pk4"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{1,5,22,1L},{1,6,12,1L},{1,6,13,1L},{1,7,8,1L}}); + + sql = "select pk4,pk3,pk1,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk4 desc,pk3 desc,pk1 desc"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{22,5,1,1L},{13,6,1,1L},{12,6,1,1L},{8,7,1,1L}}); + + sql = "select pk1,pk3,pk4,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk1 desc,pk3 desc,pk4 desc"; + rs = conn.prepareStatement(sql).executeQuery(); + assertResultSet(rs, new Object[][]{{1,7,8,1L},{1,6,13,1L},{1,6,12,1L},{1,5,22,1L}}); + } finally { + if(conn != null) { + conn.close(); } } - assertTrue(!rs.next()); } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/phoenix-core/src/it/java/org/apache/phoenix/end2end/OrderByIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/OrderByIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/OrderByIT.java index 3bce9c7..9d6a450 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/OrderByIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/OrderByIT.java @@ -30,6 +30,7 @@ import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.apache.phoenix.util.TestUtil.assertResultSet; import java.sql.Connection; import java.sql.Date; @@ -1180,21 +1181,4 @@ public class OrderByIT extends ParallelStatsDisabledIT { } } - private void assertResultSet(ResultSet rs,Object[][] rows) throws Exception { - for(int rowIndex=0;rowIndex<rows.length;rowIndex++) { - assertTrue(rs.next()); - for(int columnIndex=1;columnIndex<= rows[rowIndex].length;columnIndex++) { - Object realValue=rs.getObject(columnIndex); - Object expectedValue=rows[rowIndex][columnIndex-1]; - if(realValue==null) { - assertTrue(expectedValue==null); - } - else { - assertTrue(realValue.equals(expectedValue)); - } - } - } - assertTrue(!rs.next()); - } - } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryIT.java index 1da98d2..4a11b41 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryIT.java @@ -148,7 +148,7 @@ public class SubqueryIT extends BaseJoinIT { " PARALLEL LEFT-JOIN TABLE 0\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_SCHEMA + ".idx_item\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + " PARALLEL ANTI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + " SERVER AGGREGATE INTO DISTINCT ROWS BY \\[\"item_id\"\\]\n" + @@ -156,7 +156,7 @@ public class SubqueryIT extends BaseJoinIT { " PARALLEL LEFT-JOIN TABLE 1\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_SCHEMA + ".idx_item\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + " PARALLEL SEMI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + " SERVER AGGREGATE INTO DISTINCT ROWS BY \\[\"item_id\"\\]\n" + @@ -220,7 +220,7 @@ public class SubqueryIT extends BaseJoinIT { " PARALLEL LEFT-JOIN TABLE 0\n" + " CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + JOIN_ITEM_TABLE_FULL_NAME + " \\[1\\]\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + " CLIENT MERGE SORT\n" + " PARALLEL ANTI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + @@ -229,7 +229,7 @@ public class SubqueryIT extends BaseJoinIT { " PARALLEL LEFT-JOIN TABLE 1\n" + " CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + JOIN_ITEM_TABLE_FULL_NAME + " \\[1\\]\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + " CLIENT MERGE SORT\n" + " PARALLEL SEMI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryUsingSortMergeJoinIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryUsingSortMergeJoinIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryUsingSortMergeJoinIT.java index 9335065..665908f 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryUsingSortMergeJoinIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryUsingSortMergeJoinIT.java @@ -136,7 +136,8 @@ public class SubqueryUsingSortMergeJoinIT extends BaseJoinIT { " AND\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_SCHEMA + ".idx_item\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + + " CLIENT SORTED BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n"+ " PARALLEL ANTI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + " SERVER AGGREGATE INTO DISTINCT ROWS BY \\[\"item_id\"\\]\n" + @@ -145,7 +146,8 @@ public class SubqueryUsingSortMergeJoinIT extends BaseJoinIT { "AND\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_SCHEMA + ".idx_item\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + + " CLIENT SORTED BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n"+ " PARALLEL SEMI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + " SERVER AGGREGATE INTO DISTINCT ROWS BY \\[\"item_id\"\\]\n" + @@ -200,8 +202,9 @@ public class SubqueryUsingSortMergeJoinIT extends BaseJoinIT { " AND\n" + " CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + JOIN_ITEM_TABLE_FULL_NAME + " \\[1\\]\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + " CLIENT MERGE SORT\n" + + " CLIENT SORTED BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + " PARALLEL ANTI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + " SERVER AGGREGATE INTO DISTINCT ROWS BY \\[\"item_id\"\\]\n" + @@ -210,8 +213,9 @@ public class SubqueryUsingSortMergeJoinIT extends BaseJoinIT { "AND\n" + " CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + JOIN_ITEM_TABLE_FULL_NAME + " \\[1\\]\n" + " SERVER FILTER BY FIRST KEY ONLY\n" + - " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + + " SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY \\[\".+.0:NAME\", \".+.:item_id\"\\]\n" + " CLIENT MERGE SORT\n" + + " CLIENT SORTED BY \\[\".+.:item_id\", \".+.0:NAME\"\\]\n" + " PARALLEL SEMI-JOIN TABLE 0 \\(SKIP MERGE\\)\n" + " CLIENT PARALLEL 1-WAY FULL SCAN OVER " + JOIN_ORDER_TABLE_FULL_NAME + "\n" + " SERVER AGGREGATE INTO DISTINCT ROWS BY \\[\"item_id\"\\]\n" + http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/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 a405317..0a9e1bc 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 @@ -153,9 +153,24 @@ public class GroupByCompiler { // column and use each subsequent one in PK order). isOrderPreserving = tracker.isOrderPreserving(); orderPreservingColumnCount = tracker.getOrderPreservingColumnCount(); + if(isOrderPreserving) { + //reorder the groupby expressions following pk columns + List<Expression> newExpressions = tracker.getExpressionsFromOrderPreservingTrackInfos(); + assert newExpressions.size() == expressions.size(); + return new GroupBy.GroupByBuilder(this) + .setIsOrderPreserving(isOrderPreserving) + .setOrderPreservingColumnCount(orderPreservingColumnCount) + .setExpressions(newExpressions) + .setKeyExpressions(newExpressions) + .build(); + } } - if (isOrderPreserving || isUngroupedAggregate) { - return new GroupBy.GroupByBuilder(this).setIsOrderPreserving(isOrderPreserving).setOrderPreservingColumnCount(orderPreservingColumnCount).build(); + + if (isUngroupedAggregate) { + return new GroupBy.GroupByBuilder(this) + .setIsOrderPreserving(isOrderPreserving) + .setOrderPreservingColumnCount(orderPreservingColumnCount) + .build(); } List<Expression> expressions = Lists.newArrayListWithExpectedSize(this.expressions.size()); List<Expression> keyExpressions = expressions; http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java index dab2078..d1175f6 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java @@ -9,6 +9,7 @@ */ package org.apache.phoenix.compile; +import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.Iterator; @@ -49,6 +50,7 @@ public class OrderPreservingTracker { public final OrderPreserving orderPreserving; public final int pkPosition; public final int slotSpan; + public Expression expression; public Info(int pkPosition) { this.pkPosition = pkPosition; @@ -141,6 +143,7 @@ public class OrderPreservingTracker { return; } } + info.expression = node; orderPreservingInfos.add(info); } } @@ -153,6 +156,16 @@ public class OrderPreservingTracker { return orderPreservingColumnCount; } + public List<Expression> getExpressionsFromOrderPreservingTrackInfos() { + assert isOrderPreserving; + assert (this.orderPreservingInfos != null && this.orderPreservingInfos.size() > 0); + List<Expression> newExpressions = new ArrayList<Expression>(this.orderPreservingInfos.size()); + for(Info trackInfo : this.orderPreservingInfos) { + newExpressions.add(trackInfo.expression); + } + return newExpressions; + } + public boolean isOrderPreserving() { if (!isOrderPreserving) { return false; http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/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 bac4ee8..07bd3a9 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 @@ -98,6 +98,7 @@ import org.apache.phoenix.util.PropertiesUtil; import org.apache.phoenix.util.QueryUtil; import org.apache.phoenix.util.ScanUtil; import org.apache.phoenix.util.SchemaUtil; +import org.apache.phoenix.util.TestUtil; import org.junit.Ignore; import org.junit.Test; @@ -4850,4 +4851,113 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest { return Collections.emptyList(); } } + + @Test + public void testGroupByOrderMatchPkColumnOrder4690() throws Exception{ + this.doTestGroupByOrderMatchPkColumnOrderBug4690(false, false); + this.doTestGroupByOrderMatchPkColumnOrderBug4690(false, true); + this.doTestGroupByOrderMatchPkColumnOrderBug4690(true, false); + this.doTestGroupByOrderMatchPkColumnOrderBug4690(true, true); + } + + private void doTestGroupByOrderMatchPkColumnOrderBug4690(boolean desc ,boolean salted) throws Exception { + Connection conn = null; + try { + conn = DriverManager.getConnection(getUrl()); + String tableName = generateUniqueName(); + String sql = "create table " + tableName + "( "+ + " pk1 integer not null , " + + " pk2 integer not null, " + + " pk3 integer not null," + + " pk4 integer not null,"+ + " v integer, " + + " CONSTRAINT TEST_PK PRIMARY KEY ( "+ + "pk1 "+(desc ? "desc" : "")+", "+ + "pk2 "+(desc ? "desc" : "")+", "+ + "pk3 "+(desc ? "desc" : "")+", "+ + "pk4 "+(desc ? "desc" : "")+ + " )) "+(salted ? "SALT_BUCKETS =4" : "split on(2)"); + conn.createStatement().execute(sql); + + sql = "select pk2,pk1,count(v) from " + tableName + " group by pk2,pk1 order by pk2,pk1"; + QueryPlan queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().size() ==2); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(0).toString().equals("PK2")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(1).toString().equals("PK1")); + + sql = "select pk1,pk2,count(v) from " + tableName + " group by pk2,pk1 order by pk1,pk2"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy() == (!desc ? OrderBy.FWD_ROW_KEY_ORDER_BY : OrderBy.REV_ROW_KEY_ORDER_BY)); + + sql = "select pk2,pk1,count(v) from " + tableName + " group by pk2,pk1 order by pk2 desc,pk1 desc"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().size() ==2); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(0).toString().equals("PK2 DESC")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(1).toString().equals("PK1 DESC")); + + sql = "select pk1,pk2,count(v) from " + tableName + " group by pk2,pk1 order by pk1 desc,pk2 desc"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy() == (!desc ? OrderBy.REV_ROW_KEY_ORDER_BY : OrderBy.FWD_ROW_KEY_ORDER_BY)); + + + sql = "select pk3,pk2,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk3,pk2"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().size() == 2); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(0).toString().equals("PK3")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(1).toString().equals("PK2")); + + sql = "select pk2,pk3,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk2,pk3"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy() == (!desc ? OrderBy.FWD_ROW_KEY_ORDER_BY : OrderBy.REV_ROW_KEY_ORDER_BY)); + + sql = "select pk3,pk2,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk3 desc,pk2 desc"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().size() == 2); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(0).toString().equals("PK3 DESC")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(1).toString().equals("PK2 DESC")); + + sql = "select pk2,pk3,count(v) from " + tableName + " where pk1=1 group by pk3,pk2 order by pk2 desc,pk3 desc"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy() == (!desc ? OrderBy.REV_ROW_KEY_ORDER_BY : OrderBy.FWD_ROW_KEY_ORDER_BY)); + + + sql = "select pk4,pk3,pk1,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk4,pk3,pk1"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().size() == 3); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(0).toString().equals("PK4")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(1).toString().equals("PK3")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(2).toString().equals("PK1")); + + sql = "select pk1,pk3,pk4,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk1,pk3,pk4"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy() == (!desc ? OrderBy.FWD_ROW_KEY_ORDER_BY : OrderBy.REV_ROW_KEY_ORDER_BY)); + + sql = "select pk4,pk3,pk1,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk4 desc,pk3 desc,pk1 desc"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().size() == 3); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(0).toString().equals("PK4 DESC")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(1).toString().equals("PK3 DESC")); + assertTrue(queryPlan.getOrderBy().getOrderByExpressions().get(2).toString().equals("PK1 DESC")); + + sql = "select pk1,pk3,pk4,count(v) from " + tableName + " where pk2=9 group by pk4,pk3,pk1 order by pk1 desc,pk3 desc,pk4 desc"; + queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql); + assertTrue(queryPlan.getGroupBy().isOrderPreserving()); + assertTrue(queryPlan.getOrderBy() == (!desc ? OrderBy.REV_ROW_KEY_ORDER_BY : OrderBy.FWD_ROW_KEY_ORDER_BY)); + } finally { + if(conn != null) { + conn.close(); + } + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/153b357d/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java b/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java index a06fd69..277e257 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java @@ -1043,4 +1043,24 @@ public class TestUtil { return queryPlan; } + public static void assertResultSet(ResultSet rs,Object[][] rows) throws Exception { + for(int rowIndex=0; rowIndex < rows.length; rowIndex++) { + assertTrue("rowIndex:["+rowIndex+"] rs.next error!",rs.next()); + for(int columnIndex = 1; columnIndex <= rows[rowIndex].length; columnIndex++) { + Object realValue = rs.getObject(columnIndex); + Object expectedValue = rows[rowIndex][columnIndex-1]; + if(realValue == null) { + assertNull("rowIndex:["+rowIndex+"],columnIndex:["+columnIndex+"]",expectedValue); + } + else { + assertEquals("rowIndex:["+rowIndex+"],columnIndex:["+columnIndex+"],realValue:["+ + realValue+"],expectedValue:["+expectedValue+"]", + expectedValue, + realValue + ); + } + } + } + assertTrue(!rs.next()); + } }