Repository: phoenix Updated Branches: refs/heads/4.x-HBase-0.98 5098c168a -> d40faac9a
PHOENIX-2992 Remove ORDER BY from aggregate-only SELECT statements. Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/d40faac9 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/d40faac9 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/d40faac9 Branch: refs/heads/4.x-HBase-0.98 Commit: d40faac9abc59a19400c8c521c3f7dd429b396e5 Parents: 5098c16 Author: Lars Hofhansl <la...@apache.org> Authored: Mon Jun 13 22:34:54 2016 -0700 Committer: Lars Hofhansl <la...@apache.org> Committed: Mon Jun 13 22:34:54 2016 -0700 ---------------------------------------------------------------------- .../phoenix/end2end/DistinctPrefixFilterIT.java | 5 +++-- .../apache/phoenix/compile/GroupByCompiler.java | 8 +++----- .../apache/phoenix/compile/OrderByCompiler.java | 15 ++++++++++++--- .../phoenix/compile/WhereOptimizerTest.java | 18 +++++++++++++++++- 4 files changed, 35 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/d40faac9/phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.java index 8229243..0310183 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.java @@ -164,13 +164,14 @@ public class DistinctPrefixFilterIT extends BaseHBaseManagedTimeTableReuseIT { testPlan("SELECT DISTINCT prefix1, prefix2 FROM "+testTable+" WHERE col1 > 0.5", true); testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" HAVING COUNT(col1) > 10", false); - testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" ORDER BY COUNT(col1)", false); + testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" ORDER BY COUNT(col1)", true); + testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" ORDER BY COUNT(prefix1)", true); + testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" ORDER BY COUNT(prefix2)", true); // can't optimize the following, yet, even though it would be possible testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" HAVING COUNT(DISTINCT prefix2) > 10", false); testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" HAVING COUNT(DISTINCT prefix1) > 10", false); testPlan("SELECT COUNT(DISTINCT prefix1) / 10 FROM "+testTable, false); - testPlan("SELECT COUNT(DISTINCT prefix1) FROM "+testTable+" ORDER BY COUNT(prefix1)", false); } private void testPlan(String query, boolean optimizable) throws Exception { http://git-wip-us.apache.org/repos/asf/phoenix/blob/d40faac9/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 948b1c9..267d132 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 @@ -77,7 +77,7 @@ public class GroupByCompiler { return null; } }; - public static final GroupByCompiler.GroupBy UNGROUPED_GROUP_BY = new GroupBy(new GroupByBuilder().setIsOrderPreserving(true)) { + public static final GroupByCompiler.GroupBy UNGROUPED_GROUP_BY = new GroupBy(new GroupByBuilder().setIsOrderPreserving(true).setIsUngroupedAggregate(true)) { @Override public GroupBy compile(StatementContext context, TupleProjector tupleProjector) throws SQLException { return this; @@ -332,11 +332,9 @@ public class GroupByCompiler { // do not optimize if // 1. we were asked not to optimize // 2. there's any HAVING clause - // 3. there's any ORDER BY clause - // TODO: PHOENIX-2989 suggests some ways to optimize the latter two cases + // TODO: PHOENIX-2989 suggests some ways to optimize the latter case if (statement.getHint().hasHint(Hint.RANGE_SCAN) || - statement.getHaving() != null || - !statement.getOrderBy().isEmpty()) { + statement.getHaving() != null) { return GroupBy.UNGROUPED_GROUP_BY; } groupByNodes = Lists.newArrayListWithExpectedSize(statement.getSelect().size()); http://git-wip-us.apache.org/repos/asf/phoenix/blob/d40faac9/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java index 91fa5c8..6804375 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java @@ -91,7 +91,16 @@ public class OrderByCompiler { if (orderByNodes.isEmpty()) { return OrderBy.EMPTY_ORDER_BY; } - ExpressionCompiler compiler = new ExpressionCompiler(context, groupBy); + // for ungroupedAggregates as GROUP BY expression, check against an empty group by + ExpressionCompiler compiler; + if (groupBy.isUngroupedAggregate()) { + compiler = new ExpressionCompiler(context, GroupBy.EMPTY_GROUP_BY) { + @Override + protected Expression addExpression(Expression expression) {return expression;} + }; + } else { + compiler = new ExpressionCompiler(context, groupBy); + } // accumulate columns in ORDER BY OrderPreservingTracker tracker = new OrderPreservingTracker(context, groupBy, Ordering.ORDERED, orderByNodes.size(), tupleProjector); @@ -136,8 +145,8 @@ public class OrderByCompiler { } compiler.reset(); } - - if (orderByExpressions.isEmpty()) { + // we can remove ORDER BY clauses in case of only COUNT(DISTINCT...) clauses + if (orderByExpressions.isEmpty() || groupBy.isUngroupedAggregate()) { return OrderBy.EMPTY_ORDER_BY; } // If we're ordering by the order returned by the scan, we don't need an order by http://git-wip-us.apache.org/repos/asf/phoenix/blob/d40faac9/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java index f259373..c50975b 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java @@ -2015,5 +2015,21 @@ public class WhereOptimizerTest extends BaseConnectionlessQueryTest { assertArrayEquals(ByteUtil.concat(PVarchar.INSTANCE.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, PChar.INSTANCE.toBytes("foo"), PInteger.INSTANCE.toBytes(1), PInteger.INSTANCE.toBytes(3)), context.getScan().getStartRow()); assertArrayEquals(ByteUtil.concat(PVarchar.INSTANCE.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, PChar.INSTANCE.toBytes("foo"), PInteger.INSTANCE.toBytes(1), ByteUtil.nextKey(PInteger.INSTANCE.toBytes(3))), context.getScan().getStopRow()); } - + + @Test + public void testNoAggregatorForOrderBy() throws SQLException { + Connection conn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES)); + conn.createStatement().execute("create table test (pk1 integer not null, pk2 integer not null, constraint pk primary key (pk1,pk2))"); + StatementContext context = compileStatement("select count(distinct pk1) from test order by count(distinct pk2)"); + assertEquals(1, context.getAggregationManager().getAggregators().getAggregatorCount()); + context = compileStatement("select sum(pk1) from test order by count(distinct pk2)"); + assertEquals(1, context.getAggregationManager().getAggregators().getAggregatorCount()); + context = compileStatement("select min(pk1) from test order by count(distinct pk2)"); + assertEquals(1, context.getAggregationManager().getAggregators().getAggregatorCount()); + context = compileStatement("select max(pk1) from test order by count(distinct pk2)"); + assertEquals(1, context.getAggregationManager().getAggregators().getAggregatorCount()); + // here the ORDER BY is not optimized away + context = compileStatement("select avg(pk1) from test order by count(distinct pk2)"); + assertEquals(2, context.getAggregationManager().getAggregators().getAggregatorCount()); + } }