From b18d3800183c0719105eaf330fc4a4ab2f09ac80 Mon Sep 17 00:00:00 2001 From: lukasz-stec Date: Fri, 4 Aug 2023 12:07:36 +0200 Subject: [PATCH] Drop deprecated optimizer.use-mark-distinct Config property `optimizer.use-mark-distinct` and corresponding session property `use_mark_distinct` are removed. The `optimizer.mark-distinct-strategy` and `mark_distinct_strategy` should be used instead. --- .../io/trino/SystemSessionProperties.java | 20 +---------------- .../io/trino/sql/planner/OptimizerConfig.java | 22 ++----------------- .../io/trino/sql/planner/PlanFragmenter.java | 2 +- .../io/trino/cost/TestOptimizerConfig.java | 2 +- ...ipleDistinctAggregationToMarkDistinct.java | 17 ++------------ ...estDistinctAggregationsNoMarkDistinct.java | 4 ++-- .../main/sphinx/admin/properties-optimizer.md | 5 ----- .../plugin/jdbc/BaseJdbcConnectorTest.java | 6 ++--- .../testing/AbstractTestAggregations.java | 4 ++-- 9 files changed, 14 insertions(+), 68 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/SystemSessionProperties.java b/core/trino-main/src/main/java/io/trino/SystemSessionProperties.java index 2f50f160ecd8..af1c6bb6823b 100644 --- a/core/trino-main/src/main/java/io/trino/SystemSessionProperties.java +++ b/core/trino-main/src/main/java/io/trino/SystemSessionProperties.java @@ -119,7 +119,6 @@ public final class SystemSessionProperties public static final String USE_PARTIAL_TOPN = "use_partial_topn"; public static final String USE_PARTIAL_DISTINCT_LIMIT = "use_partial_distinct_limit"; public static final String MAX_RECURSION_DEPTH = "max_recursion_depth"; - public static final String USE_MARK_DISTINCT = "use_mark_distinct"; public static final String MARK_DISTINCT_STRATEGY = "mark_distinct_strategy"; public static final String PREFER_PARTIAL_AGGREGATION = "prefer_partial_aggregation"; public static final String OPTIMIZE_TOP_N_RANKING = "optimize_top_n_ranking"; @@ -573,11 +572,6 @@ public SystemSessionProperties( false, value -> validateIntegerValue(value, MAX_RECURSION_DEPTH, 1, false), object -> object), - booleanProperty( - USE_MARK_DISTINCT, - "Implement DISTINCT aggregations using MarkDistinct", - optimizerConfig.isUseMarkDistinct(), - false), enumProperty( MARK_DISTINCT_STRATEGY, "", @@ -1362,19 +1356,7 @@ public static int getFilterAndProjectMinOutputPageRowCount(Session session) public static MarkDistinctStrategy markDistinctStrategy(Session session) { - MarkDistinctStrategy markDistinctStrategy = session.getSystemProperty(MARK_DISTINCT_STRATEGY, MarkDistinctStrategy.class); - if (markDistinctStrategy != null) { - // mark_distinct_strategy is set, so it takes precedence over use_mark_distinct - return markDistinctStrategy; - } - - Boolean useMarkDistinct = session.getSystemProperty(USE_MARK_DISTINCT, Boolean.class); - if (useMarkDistinct == null) { - // both mark_distinct_strategy and use_mark_distinct have default null values, use AUTOMATIC - return MarkDistinctStrategy.AUTOMATIC; - } - // use_mark_distinct is set but mark_distinct_strategy is not, map use_mark_distinct to mark_distinct_strategy - return useMarkDistinct ? MarkDistinctStrategy.AUTOMATIC : MarkDistinctStrategy.NONE; + return session.getSystemProperty(MARK_DISTINCT_STRATEGY, MarkDistinctStrategy.class); } public static boolean preferPartialAggregation(Session session) diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/OptimizerConfig.java b/core/trino-main/src/main/java/io/trino/sql/planner/OptimizerConfig.java index df48e79888b9..b6216bffa8b9 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/OptimizerConfig.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/OptimizerConfig.java @@ -29,7 +29,7 @@ import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.MINUTES; -@DefunctConfig({"adaptive-partial-aggregation.min-rows", "preferred-write-partitioning-min-number-of-partitions"}) +@DefunctConfig({"adaptive-partial-aggregation.min-rows", "preferred-write-partitioning-min-number-of-partitions", "optimizer.use-mark-distinct"}) public class OptimizerConfig { private double cpuCostWeight = 75; @@ -64,10 +64,7 @@ public class OptimizerConfig private boolean optimizeHashGeneration = true; private boolean pushTableWriteThroughUnion = true; private boolean dictionaryAggregation; - @Nullable - private Boolean useMarkDistinct; - @Nullable - private MarkDistinctStrategy markDistinctStrategy; + private MarkDistinctStrategy markDistinctStrategy = MarkDistinctStrategy.AUTOMATIC; private boolean preferPartialAggregation = true; private boolean pushAggregationThroughOuterJoin = true; private boolean enableIntermediateAggregations; @@ -473,21 +470,6 @@ public OptimizerConfig setOptimizeMetadataQueries(boolean optimizeMetadataQuerie return this; } - @Deprecated - @Nullable - public Boolean isUseMarkDistinct() - { - return useMarkDistinct; - } - - @Deprecated - @LegacyConfig(value = "optimizer.use-mark-distinct", replacedBy = "optimizer.mark-distinct-strategy") - public OptimizerConfig setUseMarkDistinct(Boolean value) - { - this.useMarkDistinct = value; - return this; - } - @Nullable public MarkDistinctStrategy getMarkDistinctStrategy() { diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/PlanFragmenter.java b/core/trino-main/src/main/java/io/trino/sql/planner/PlanFragmenter.java index 4f780d18cbb9..bdcd5ff9204f 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/PlanFragmenter.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/PlanFragmenter.java @@ -88,7 +88,7 @@ public class PlanFragmenter { private static final String TOO_MANY_STAGES_MESSAGE = "" + - "If the query contains multiple aggregates with DISTINCT over different columns, please set the 'use_mark_distinct' session property to false. " + + "If the query contains multiple aggregates with DISTINCT over different columns, please set the 'mark_distinct_strategy' session property to 'none'. " + "If the query contains WITH clauses that are referenced more than once, please create temporary table(s) for the queries in those clauses."; private final Metadata metadata; diff --git a/core/trino-main/src/test/java/io/trino/cost/TestOptimizerConfig.java b/core/trino-main/src/test/java/io/trino/cost/TestOptimizerConfig.java index 243203aa5e53..162fe6f9b392 100644 --- a/core/trino-main/src/test/java/io/trino/cost/TestOptimizerConfig.java +++ b/core/trino-main/src/test/java/io/trino/cost/TestOptimizerConfig.java @@ -70,7 +70,7 @@ public void testDefaults() .setPushAggregationThroughOuterJoin(true) .setPushPartialAggregationThroughJoin(false) .setPreAggregateCaseAggregationsEnabled(true) - .setMarkDistinctStrategy(null) + .setMarkDistinctStrategy(OptimizerConfig.MarkDistinctStrategy.AUTOMATIC) .setPreferPartialAggregation(true) .setOptimizeTopNRanking(true) .setDistributedSortEnabled(true) diff --git a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestMultipleDistinctAggregationToMarkDistinct.java b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestMultipleDistinctAggregationToMarkDistinct.java index edaf3fbf31e6..4b5950c0a51b 100644 --- a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestMultipleDistinctAggregationToMarkDistinct.java +++ b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestMultipleDistinctAggregationToMarkDistinct.java @@ -31,7 +31,6 @@ import static io.trino.SystemSessionProperties.MARK_DISTINCT_STRATEGY; import static io.trino.SystemSessionProperties.OPTIMIZE_DISTINCT_AGGREGATIONS; import static io.trino.SystemSessionProperties.TASK_CONCURRENCY; -import static io.trino.SystemSessionProperties.USE_MARK_DISTINCT; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.sql.planner.assertions.PlanMatchPattern.aggregation; import static io.trino.sql.planner.assertions.PlanMatchPattern.functionCall; @@ -230,30 +229,18 @@ public void testAggregationNDV() .overrideStats(aggregationNodeId.toString(), PlanNodeStatsEstimate.builder().setOutputRowCount(1000 * clusterThreadCount).build()) .doesNotFire(); - // big NDV, mark_distinct_strategy = always, use_mark_distinct = null + // big NDV, mark_distinct_strategy = always tester().assertThat(new MultipleDistinctAggregationToMarkDistinct(TASK_COUNT_ESTIMATOR)) .on(plan) .setSystemProperty(MARK_DISTINCT_STRATEGY, "always") .overrideStats(aggregationNodeId.toString(), PlanNodeStatsEstimate.builder().setOutputRowCount(1000 * clusterThreadCount).build()) .matches(expectedMarkDistinct); - // big NDV, mark_distinct_strategy = null, use_mark_distinct = true - tester().assertThat(new MultipleDistinctAggregationToMarkDistinct(TASK_COUNT_ESTIMATOR)) - .on(plan) - .setSystemProperty(USE_MARK_DISTINCT, "true") - .overrideStats(aggregationNodeId.toString(), PlanNodeStatsEstimate.builder().setOutputRowCount(1000 * clusterThreadCount).build()) - .doesNotFire(); - // small NDV, mark_distinct_strategy = none, use_mark_distinct = null + // small NDV, mark_distinct_strategy = none tester().assertThat(new MultipleDistinctAggregationToMarkDistinct(TASK_COUNT_ESTIMATOR)) .on(plan) .setSystemProperty(MARK_DISTINCT_STRATEGY, "none") .overrideStats(aggregationNodeId.toString(), PlanNodeStatsEstimate.builder().setOutputRowCount(2 * clusterThreadCount).build()) .doesNotFire(); - // small NDV, mark_distinct_strategy = null, use_mark_distinct = false - tester().assertThat(new MultipleDistinctAggregationToMarkDistinct(TASK_COUNT_ESTIMATOR)) - .on(plan) - .setSystemProperty(USE_MARK_DISTINCT, "false") - .overrideStats(aggregationNodeId.toString(), PlanNodeStatsEstimate.builder().setOutputRowCount(2 * clusterThreadCount).build()) - .doesNotFire(); // big NDV but on multiple grouping keys tester().assertThat(new MultipleDistinctAggregationToMarkDistinct(TASK_COUNT_ESTIMATOR)) diff --git a/core/trino-main/src/test/java/io/trino/sql/query/TestDistinctAggregationsNoMarkDistinct.java b/core/trino-main/src/test/java/io/trino/sql/query/TestDistinctAggregationsNoMarkDistinct.java index fcf33be3be8c..6cc41666d07e 100644 --- a/core/trino-main/src/test/java/io/trino/sql/query/TestDistinctAggregationsNoMarkDistinct.java +++ b/core/trino-main/src/test/java/io/trino/sql/query/TestDistinctAggregationsNoMarkDistinct.java @@ -15,7 +15,7 @@ import org.junit.jupiter.api.BeforeAll; -import static io.trino.SystemSessionProperties.USE_MARK_DISTINCT; +import static io.trino.SystemSessionProperties.MARK_DISTINCT_STRATEGY; import static io.trino.testing.TestingSession.testSessionBuilder; public class TestDistinctAggregationsNoMarkDistinct @@ -26,7 +26,7 @@ public class TestDistinctAggregationsNoMarkDistinct public void init() { assertions = new QueryAssertions(testSessionBuilder() - .setSystemProperty(USE_MARK_DISTINCT, "false") + .setSystemProperty(MARK_DISTINCT_STRATEGY, "none") .build()); } } diff --git a/docs/src/main/sphinx/admin/properties-optimizer.md b/docs/src/main/sphinx/admin/properties-optimizer.md index 97d93d8f603b..9fc94c6ae6a1 100644 --- a/docs/src/main/sphinx/admin/properties-optimizer.md +++ b/docs/src/main/sphinx/admin/properties-optimizer.md @@ -52,11 +52,6 @@ aggregations or for mix of distinct and non-distinct aggregations. `AUTOMATIC` limits the use of `MarkDistinct` only for cases with limited concurrency (global or small cardinality aggregations), where direct distinct aggregation implementation cannot utilize CPU efficiently. -`optimizer.mark-distinct-strategy` overrides, if set, the deprecated -`optimizer.use-mark-distinct`. If `optimizer.mark-distinct-strategy` is not -set, but `optimizer.use-mark-distinct` is then `optimizer.use-mark-distinct` -is mapped to `optimizer.mark-distinct-strategy` with value `true` mapped to -`AUTOMATIC` and value `false` mapped to `NONE`. ## `optimizer.push-aggregation-through-outer-join` diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java index f24aad871660..493df6d69b31 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java @@ -62,7 +62,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.MoreCollectors.toOptional; import static io.airlift.concurrent.Threads.daemonThreadsNamed; -import static io.trino.SystemSessionProperties.USE_MARK_DISTINCT; +import static io.trino.SystemSessionProperties.MARK_DISTINCT_STRATEGY; import static io.trino.execution.querystats.PlanOptimizersStatsCollector.createPlanOptimizersStatsCollector; import static io.trino.plugin.jdbc.JdbcDynamicFilteringSessionProperties.DYNAMIC_FILTERING_ENABLED; import static io.trino.plugin.jdbc.JdbcDynamicFilteringSessionProperties.DYNAMIC_FILTERING_WAIT_TIMEOUT; @@ -470,7 +470,7 @@ public void testDistinctAggregationPushdown() } Session withMarkDistinct = Session.builder(getSession()) - .setSystemProperty(USE_MARK_DISTINCT, "true") + .setSystemProperty(MARK_DISTINCT_STRATEGY, "always") .build(); // distinct aggregation assertThat(query(withMarkDistinct, "SELECT count(DISTINCT regionkey) FROM nation")).isFullyPushedDown(); @@ -506,7 +506,7 @@ public void testDistinctAggregationPushdown() node(MarkDistinctNode.class, node(ExchangeNode.class, node(ExchangeNode.class, node(ProjectNode.class, node(TableScanNode.class)))))); Session withoutMarkDistinct = Session.builder(getSession()) - .setSystemProperty(USE_MARK_DISTINCT, "false") + .setSystemProperty(MARK_DISTINCT_STRATEGY, "none") .build(); // distinct aggregation assertThat(query(withoutMarkDistinct, "SELECT count(DISTINCT regionkey) FROM nation")).isFullyPushedDown(); diff --git a/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestAggregations.java b/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestAggregations.java index 0da173541219..1eb273fb746c 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestAggregations.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestAggregations.java @@ -21,7 +21,7 @@ import java.util.List; -import static io.trino.SystemSessionProperties.USE_MARK_DISTINCT; +import static io.trino.SystemSessionProperties.MARK_DISTINCT_STRATEGY; import static io.trino.testing.MaterializedResult.resultBuilder; import static io.trino.testing.QueryAssertions.assertEqualsIgnoreOrder; import static org.testng.Assert.assertEquals; @@ -251,7 +251,7 @@ public void testDistinctGroupBy() assertQuery(query); assertQuery( Session.builder(getSession()) - .setSystemProperty(USE_MARK_DISTINCT, "false") + .setSystemProperty(MARK_DISTINCT_STRATEGY, "none") .build(), query); }