Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop deprecated optimizer.use-mark-distinct #18540

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
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
lukasz-stec marked this conversation as resolved.
Show resolved Hide resolved
`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