diff --git a/presto-benchmark/src/main/java/com/facebook/presto/benchmark/SqlRemoveRedundantDistinctAggregationBenchmarks.java b/presto-benchmark/src/main/java/com/facebook/presto/benchmark/SqlRemoveRedundantDistinctAggregationBenchmarks.java new file mode 100644 index 0000000000000..88a7e9104ee90 --- /dev/null +++ b/presto-benchmark/src/main/java/com/facebook/presto/benchmark/SqlRemoveRedundantDistinctAggregationBenchmarks.java @@ -0,0 +1,44 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.benchmark; + +import com.facebook.airlift.log.Logger; +import com.facebook.presto.testing.LocalQueryRunner; +import com.google.common.collect.ImmutableMap; +import org.intellij.lang.annotations.Language; + +import java.util.Map; + +import static com.facebook.presto.benchmark.BenchmarkQueryRunner.createLocalQueryRunner; + +public class SqlRemoveRedundantDistinctAggregationBenchmarks + extends AbstractSqlBenchmark +{ + private static final Logger LOGGER = Logger.get(SqlRewriteConditionalAggregationBenchmarks.class); + + public SqlRemoveRedundantDistinctAggregationBenchmarks(LocalQueryRunner localQueryRunner, @Language("SQL") String sql) + { + super(localQueryRunner, "remove_redundant_distinct_aggregation", 10, 20, sql); + } + + public static void main(String[] args) + { + Map disableOptimization = ImmutableMap.of("remove_redundant_distinct_aggregation_enabled", "false"); + String sql = "select distinct orderkey, partkey, suppkey, avg(extendedprice) from lineitem group by orderkey, partkey, suppkey"; + LOGGER.info("Without optimization"); + new SqlRemoveRedundantDistinctAggregationBenchmarks(createLocalQueryRunner(disableOptimization), sql).runBenchmark(new SimpleLineBenchmarkResultWriter(System.out)); + LOGGER.info("With optimization"); + new SqlRemoveRedundantDistinctAggregationBenchmarks(createLocalQueryRunner(), sql).runBenchmark(new SimpleLineBenchmarkResultWriter(System.out)); + } +} diff --git a/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java b/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java index ef257fba5ff06..46400db12ae95 100644 --- a/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java +++ b/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java @@ -248,6 +248,7 @@ public final class SystemSessionProperties public static final String QUICK_DISTINCT_LIMIT_ENABLED = "quick_distinct_limit_enabled"; public static final String OPTIMIZE_CONDITIONAL_AGGREGATION_ENABLED = "optimize_conditional_aggregation_enabled"; public static final String ANALYZER_TYPE = "analyzer_type"; + public static final String REMOVE_REDUNDANT_DISTINCT_AGGREGATION_ENABLED = "remove_redundant_distinct_aggregation_enabled"; // TODO: Native execution related session properties that are temporarily put here. They will be relocated in the future. public static final String NATIVE_SIMPLIFIED_EXPRESSION_EVALUATION_ENABLED = "simplified_expression_evaluation_enabled"; @@ -1409,6 +1410,11 @@ public SystemSessionProperties( OPTIMIZE_CONDITIONAL_AGGREGATION_ENABLED, "Enable rewriting IF(condition, AGG(x)) to AGG(x) with condition included in mask", featuresConfig.isOptimizeConditionalAggregationEnabled(), + false), + booleanProperty( + REMOVE_REDUNDANT_DISTINCT_AGGREGATION_ENABLED, + "Enable removing distinct aggregation node if input is already distinct", + featuresConfig.isRemoveRedundantDistinctAggregationEnabled(), false)); } @@ -2359,4 +2365,9 @@ public static boolean isOptimizeConditionalAggregationEnabled(Session session) { return session.getSystemProperty(OPTIMIZE_CONDITIONAL_AGGREGATION_ENABLED, Boolean.class); } + + public static boolean isRemoveRedundantDistinctAggregationEnabled(Session session) + { + return session.getSystemProperty(REMOVE_REDUNDANT_DISTINCT_AGGREGATION_ENABLED, Boolean.class); + } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java index 4dc5d11d0e984..fd74530aab7a5 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java @@ -239,6 +239,7 @@ public class FeaturesConfig private String nativeExecutionExecutablePath = "./presto_server"; private boolean randomizeOuterJoinNullKey; private boolean isOptimizeConditionalAggregationEnabled; + private boolean isRemoveRedundantDistinctAggregationEnabled = true; public enum PartitioningPrecisionStrategy { @@ -2266,4 +2267,17 @@ public FeaturesConfig setOptimizeConditionalAggregationEnabled(boolean isOptimiz this.isOptimizeConditionalAggregationEnabled = isOptimizeConditionalAggregationEnabled; return this; } + + public boolean isRemoveRedundantDistinctAggregationEnabled() + { + return isRemoveRedundantDistinctAggregationEnabled; + } + + @Config("optimizer.remove-redundant-distinct-aggregation-enabled") + @ConfigDescription("Enable removing distinct aggregation node if input is already distinct") + public FeaturesConfig setRemoveRedundantDistinctAggregationEnabled(boolean isRemoveRedundantDistinctAggregationEnabled) + { + this.isRemoveRedundantDistinctAggregationEnabled = isRemoveRedundantDistinctAggregationEnabled; + return this; + } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java index 3524f57de8471..78f46587727ae 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java @@ -145,6 +145,7 @@ import com.facebook.presto.sql.planner.optimizations.PruneUnreferencedOutputs; import com.facebook.presto.sql.planner.optimizations.PushdownSubfields; import com.facebook.presto.sql.planner.optimizations.RandomizeNullKeyInOuterJoin; +import com.facebook.presto.sql.planner.optimizations.RemoveRedundantDistinctAggregation; import com.facebook.presto.sql.planner.optimizations.ReplicateSemiJoinInDelete; import com.facebook.presto.sql.planner.optimizations.RewriteIfOverAggregation; import com.facebook.presto.sql.planner.optimizations.SetFlatteningOptimizer; @@ -602,6 +603,8 @@ public PlanOptimizers( .add(new InlineProjections(metadata.getFunctionAndTypeManager())) .build())); + builder.add(new RemoveRedundantDistinctAggregation()); + if (!forceSingleNode) { builder.add(new ReplicateSemiJoinInDelete()); // Must run before AddExchanges builder.add(new IterativeOptimizer( diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/RemoveRedundantDistinctAggregation.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/RemoveRedundantDistinctAggregation.java new file mode 100644 index 0000000000000..c95651d448dbc --- /dev/null +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/RemoveRedundantDistinctAggregation.java @@ -0,0 +1,142 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.sql.planner.optimizations; + +import com.facebook.presto.Session; +import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.plan.AggregationNode; +import com.facebook.presto.spi.plan.PlanNode; +import com.facebook.presto.spi.plan.PlanNodeIdAllocator; +import com.facebook.presto.spi.plan.ProjectNode; +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.PlanVariableAllocator; +import com.facebook.presto.sql.planner.TypeProvider; +import com.facebook.presto.sql.planner.plan.InternalPlanVisitor; +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Set; + +import static com.facebook.presto.SystemSessionProperties.isRemoveRedundantDistinctAggregationEnabled; +import static com.facebook.presto.spi.plan.AggregationNode.isDistinct; +import static com.facebook.presto.sql.planner.plan.ChildReplacer.replaceChildren; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static java.util.Objects.requireNonNull; + +/** + * Remove the redundant distinct if output is already distinct. + * For example, for query select distinct k, sum(x) from table group by k + * The plan will change + *

+ * From: + * 

+ * - Aggregation group by k, sum
+ *   - Aggregation (sum <- AGG(x)) group by k
+ *
+ * To: + * 
+ * - Aggregation (sum <- AGG(x)) group by k
+ *
+ *

+ */ +public class RemoveRedundantDistinctAggregation + implements PlanOptimizer +{ + @Override + public PlanNode optimize(PlanNode plan, Session session, TypeProvider types, PlanVariableAllocator variableAllocator, PlanNodeIdAllocator idAllocator, WarningCollector warningCollector) + { + if (isRemoveRedundantDistinctAggregationEnabled(session)) { + PlanWithProperties result = new RemoveRedundantDistinctAggregation.Rewriter().accept(plan); + return result.getNode(); + } + return plan; + } + + private static class PlanWithProperties + { + private final PlanNode node; + // Variables in each set combines to be distinct in the output of the plan node. + private final List> distinctVariableSet; + + public PlanWithProperties(PlanNode node, List> distinctVariableSet) + { + this.node = requireNonNull(node, "node is null"); + this.distinctVariableSet = requireNonNull(distinctVariableSet, "StreamProperties is null"); + } + + public PlanNode getNode() + { + return node; + } + + public List> getProperties() + { + return distinctVariableSet; + } + } + + private static class Rewriter + extends InternalPlanVisitor + { + @Override + public PlanWithProperties visitPlan(PlanNode node, Void context) + { + // For nodes such as join, unnest etc. the distinct properties may be violated, hence pass empty list for these cases. + return planAndRecplace(node, false); + } + + @Override + public PlanWithProperties visitAggregation(AggregationNode node, Void context) + { + PlanWithProperties child = accept(node.getSource()); + if (isDistinct(node) && child.getProperties().stream().anyMatch(node.getGroupingKeys()::containsAll)) { + return child; + } + ImmutableList.Builder> properties = ImmutableList.builder(); + // Only do it for aggregations with one single grouping set + if (node.getGroupingSetCount() == 1 && !node.getGroupingKeys().isEmpty()) { + properties.add(node.getGroupingKeys().stream().collect(toImmutableSet())); + } + PlanNode newAggregation = node.replaceChildren(ImmutableList.of(child.getNode())); + return new PlanWithProperties(newAggregation, properties.build()); + } + + @Override + public PlanWithProperties visitProject(ProjectNode node, Void context) + { + return planAndRecplace(node, true); + } + + private PlanWithProperties accept(PlanNode node) + { + PlanWithProperties result = node.accept(this, null); + return new PlanWithProperties( + result.getNode().assignStatsEquivalentPlanNode(node.getStatsEquivalentPlanNode()), + result.getProperties()); + } + + private PlanWithProperties planAndRecplace(PlanNode node, boolean passProperties) + { + List children = node.getSources().stream().map(this::accept).collect(toImmutableList()); + PlanNode result = replaceChildren(node, children.stream().map(PlanWithProperties::getNode).collect(toImmutableList())); + if (!passProperties) { + return new PlanWithProperties(result, ImmutableList.of()); + } + ImmutableList.Builder> properties = ImmutableList.builder(); + children.stream().map(PlanWithProperties::getProperties).forEach(properties::addAll); + return new PlanWithProperties(result, properties.build()); + } + } +} diff --git a/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java b/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java index 28a959a7112e7..6c2d72bd6572c 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java @@ -211,7 +211,8 @@ public void testDefaults() .setNativeExecutionEnabled(false) .setNativeExecutionExecutablePath("./presto_server") .setRandomizeOuterJoinNullKeyEnabled(false) - .setOptimizeConditionalAggregationEnabled(false)); + .setOptimizeConditionalAggregationEnabled(false) + .setRemoveRedundantDistinctAggregationEnabled(true)); } @Test @@ -372,6 +373,7 @@ public void testExplicitPropertyMappings() .put("native-execution-executable-path", "/bin/echo") .put("optimizer.randomize-outer-join-null-key", "true") .put("optimizer.optimize-conditional-aggregation-enabled", "true") + .put("optimizer.remove-redundant-distinct-aggregation-enabled", "false") .build(); FeaturesConfig expected = new FeaturesConfig() @@ -529,7 +531,8 @@ public void testExplicitPropertyMappings() .setNativeExecutionEnabled(true) .setNativeExecutionExecutablePath("/bin/echo") .setRandomizeOuterJoinNullKeyEnabled(true) - .setOptimizeConditionalAggregationEnabled(true); + .setOptimizeConditionalAggregationEnabled(true) + .setRemoveRedundantDistinctAggregationEnabled(false); assertFullMapping(properties, expected); } diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/optimizations/TestRemoveRedundantDistinctAggregation.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/optimizations/TestRemoveRedundantDistinctAggregation.java new file mode 100644 index 0000000000000..4fb9e5296feca --- /dev/null +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/optimizations/TestRemoveRedundantDistinctAggregation.java @@ -0,0 +1,258 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.sql.planner.optimizations; + +import com.facebook.presto.sql.planner.assertions.BasePlanTest; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import org.testng.annotations.Test; + +import static com.facebook.presto.spi.plan.AggregationNode.Step.FINAL; +import static com.facebook.presto.spi.plan.AggregationNode.Step.PARTIAL; +import static com.facebook.presto.spi.plan.AggregationNode.Step.SINGLE; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.aggregation; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.anyTree; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.equiJoinClause; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.expression; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.functionCall; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.groupingSet; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.join; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.output; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.project; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.tableScan; +import static com.facebook.presto.sql.planner.plan.JoinNode.Type.INNER; + +public class TestRemoveRedundantDistinctAggregation + extends BasePlanTest +{ + @Test + public void testDistinctOverSingleGroupBy() + { + assertPlan("SELECT DISTINCT orderpriority, SUM(totalprice) FROM orders GROUP BY orderpriority", + output( + project( + aggregation( + ImmutableMap.of("finalsum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority"))))))))); + } + + @Test + public void testDistinctOverSingleGroupingSet() + { + assertPlan("SELECT DISTINCT orderpriority, SUM(totalprice) FROM orders GROUP BY GROUPING SETS ((orderpriority))", + output( + project( + aggregation( + ImmutableMap.of("finalsum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority"))))))))); + } + + // Should not trigger + @Test + public void testDistinctOverMultipleGroupingSet() + { + assertPlan("SELECT DISTINCT orderpriority, orderstatus, SUM(totalprice) FROM orders GROUP BY GROUPING SETS ((orderpriority), (orderstatus))", + output( + anyTree( + aggregation( + ImmutableMap.of(), + anyTree( + aggregation( + ImmutableMap.of("finalsum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + groupingSet( + ImmutableList.of(ImmutableList.of("orderpriority"), ImmutableList.of("orderstatus")), + ImmutableMap.of("totalprice", "totalprice"), + "groupid", + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority", "orderstatus", "orderstatus")))))))))))); + } + + @Test + public void testDistinctWithRandom() + { + assertPlan("SELECT DISTINCT orderpriority, random(), SUM(totalprice) FROM orders GROUP BY orderpriority", + output( + project( + aggregation( + ImmutableMap.of("finalsum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority"))))))))); + } + + @Test + public void testDistinctWithRandomFromGroupBy() + { + assertPlan("SELECT DISTINCT orderpriority, random(), sum from (select orderpriority, SUM(totalprice) as sum FROM orders GROUP BY orderpriority)", + output( + project( + aggregation( + ImmutableMap.of("finalsum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority"))))))))); + } + + // Does not trigger optimization + @Test + public void testDistinctOverSubsetOfGroupBy() + { + assertPlan("SELECT DISTINCT orderpriority, sum FROM (SELECT orderpriority, orderstatus, SUM(totalprice) AS sum FROM orders GROUP BY orderpriority, orderstatus)", + output( + project( + aggregation( + ImmutableMap.of(), + project( + aggregation( + ImmutableMap.of("finalsum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority", "orderstatus", "orderstatus"))))))))))); + } + + // Does not trigger + @Test + public void testDistinctExpressionWithGroupBy() + { + assertPlan("SELECT DISTINCT orderkey+1 AS orderkey, sum FROM (SELECT orderkey, SUM(totalprice) AS sum FROM orders GROUP BY orderkey)", + output( + project( + aggregation( + ImmutableMap.of(), + FINAL, + anyTree( + aggregation( + ImmutableMap.of(), + PARTIAL, + anyTree( + project( + ImmutableMap.of("expr", expression("orderkey+1")), + aggregation( + ImmutableMap.of("sum", functionCall("sum", ImmutableList.of("totalprice"))), + SINGLE, + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderkey", "orderkey"))))))))))); + } + + // Does not trigger + @Test + public void testJoinWithGroupByKey() + { + assertPlan("select distinct orderkey, avg, tax from (select orderkey, sum(totalprice) avg from orders group by orderkey) as t1 join lineitem using(orderkey)", + output( + project( + aggregation( + ImmutableMap.of(), + anyTree( + join( + INNER, + ImmutableList.of(equiJoinClause("l_orderkey", "orderkey")), + project( + ImmutableMap.of(), + tableScan("lineitem", ImmutableMap.of("l_orderkey", "orderkey", "tax", "tax"))), + project( + aggregation( + ImmutableMap.of("finallsum", functionCall("sum", ImmutableList.of("partialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("partialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderkey", "orderkey")))))))))))); + } + + // Does not trigger + @Test + public void testJoinWithGroupByOnDifferentKey() + { + assertPlan("select distinct orderstatus, orderkey from (select orderstatus, max_by(orderkey, totalprice) orderkey from orders group by orderstatus) as t1 join lineitem using(orderkey)", + output( + project( + aggregation( + ImmutableMap.of(), + anyTree( + join( + INNER, + ImmutableList.of(equiJoinClause("max_by", "orderkey_10")), + project( + aggregation( + ImmutableMap.of("max_by", functionCall("max_by", ImmutableList.of("max_by_24"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("max_by_24", functionCall("max_by", ImmutableList.of("orderkey", "totalprice"))), + PARTIAL, + project( + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderkey", "orderkey"))))))), + anyTree( + project( + ImmutableMap.of(), + tableScan("lineitem", ImmutableMap.of("orderkey_10", "orderkey")))))))))); + } + + @Test + public void testAggregationOverDistinct() + { + assertPlan("select orderstatus, max_by(orderpriority, sum) from (select distinct orderstatus, orderpriority, sum(totalprice) as sum from orders group by orderstatus, orderpriority) group by orderstatus", + output( + project( + aggregation( + ImmutableMap.of("max_by", functionCall("max_by", ImmutableList.of("orderpriority", "sum"))), + project( + aggregation( + ImmutableMap.of("sum", functionCall("sum", ImmutableList.of("paritialsum"))), + FINAL, + anyTree( + aggregation( + ImmutableMap.of("paritialsum", functionCall("sum", ImmutableList.of("totalprice"))), + PARTIAL, + project( + ImmutableMap.of(), + tableScan("orders", ImmutableMap.of("totalprice", "totalprice", "orderpriority", "orderpriority", "orderstatus", "orderstatus"))))))))))); + } +} diff --git a/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestAggregations.java b/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestAggregations.java index 3211a0aeadab1..83bd9fbe68302 100644 --- a/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestAggregations.java +++ b/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestAggregations.java @@ -1364,6 +1364,17 @@ public void testGroupedRow() "SELECT 15000, 15000"); } + @Test + public void testRemoveRedundantDistinctOverGroupBy() + { + String trigger = "SELECT DISTINCT suppkey, COUNT(*) FROM lineitem GROUP BY suppkey"; + assertQuery(trigger, trigger); + + String doNotTrigger = "SELECT DISTINCT suppkey, cnt FROM (SELECT suppkey, COUNT(*) AS cnt FROM lineitem GROUP BY suppkey " + + "UNION ALL SELECT suppkey, COUNT(*) AS cnt FROM lineitem GROUP BY suppkey) order by 1, 2"; + assertQuery(doNotTrigger, doNotTrigger); + } + @DataProvider(name = "getType") protected Object[][] getDigests() {