Skip to content

Commit

Permalink
Drop deprecated optimizer.use-mark-distinct
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lukasz-stec authored and sopel39 committed Aug 9, 2023
1 parent bdc2799 commit 1e7c3cf
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
"",
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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;
Expand Down Expand Up @@ -65,10 +65,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;
Expand Down Expand Up @@ -475,21 +472,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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
}
}
5 changes: 0 additions & 5 deletions docs/src/main/sphinx/admin/properties-optimizer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 1e7c3cf

Please sign in to comment.