From commits-return-20605-archive-asf-public=cust-asf.ponee.io@phoenix.apache.org Wed Apr 18 12:04:36 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 65C9918064E for ; Wed, 18 Apr 2018 12:04:35 +0200 (CEST) Received: (qmail 20554 invoked by uid 500); 18 Apr 2018 10:04:34 -0000 Mailing-List: contact commits-help@phoenix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@phoenix.apache.org Delivered-To: mailing list commits@phoenix.apache.org Received: (qmail 20545 invoked by uid 99); 18 Apr 2018 10:04:34 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 18 Apr 2018 10:04:34 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 48CE7E78E2; Wed, 18 Apr 2018 10:04:34 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: chenglei@apache.org To: commits@phoenix.apache.org Message-Id: <4b0d4707e60746fe914d27bf3e198620@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: phoenix git commit: PHOENIX-4690 GroupBy expressions should follow the order of PK Columns if GroupBy is orderPreserving Date: Wed, 18 Apr 2018 10:04:34 +0000 (UTC) Repository: phoenix Updated Branches: refs/heads/4.x-cdh5.12 157139688 -> 0860dec72 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/0860dec7 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/0860dec7 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/0860dec7 Branch: refs/heads/4.x-cdh5.12 Commit: 0860dec72b23a620a21b6b9f17d90cf45b2ed75e Parents: 1571396 Author: chenglei Authored: Wed Apr 18 18:04:09 2018 +0800 Committer: chenglei Committed: Wed Apr 18 18:04:09 2018 +0800 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/AggregateIT.java | 109 +++++++++++++++--- .../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, 268 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/0860dec7/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 fd1d660..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 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 expressions = Lists.newArrayListWithExpectedSize(this.expressions.size()); List keyExpressions = expressions; http://git-wip-us.apache.org/repos/asf/phoenix/blob/0860dec7/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 getExpressionsFromOrderPreservingTrackInfos() { + assert isOrderPreserving; + assert (this.orderPreservingInfos != null && this.orderPreservingInfos.size() > 0); + List newExpressions = new ArrayList(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/0860dec7/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/0860dec7/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()); + } }