From 2edc8bc468dcf11891b96984d47426f545c5e7a0 Mon Sep 17 00:00:00 2001 From: Feilong Liu Date: Fri, 6 Dec 2024 09:17:27 -0800 Subject: [PATCH] Add session property for connector optimizer to work on values node --- .../facebook/presto/SystemSessionProperties.java | 10 ++++++++++ .../presto/sql/analyzer/FeaturesConfig.java | 14 ++++++++++++++ .../optimizations/ApplyConnectorOptimization.java | 12 ++++++++---- .../presto/sql/analyzer/TestFeaturesConfig.java | 3 +++ 4 files changed, 35 insertions(+), 4 deletions(-) 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 f66aeaf89a2ee..28165edd441fe 100644 --- a/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java +++ b/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java @@ -324,6 +324,7 @@ public final class SystemSessionProperties public static final String OPTIMIZER_USE_HISTOGRAMS = "optimizer_use_histograms"; public static final String WARN_ON_COMMON_NAN_PATTERNS = "warn_on_common_nan_patterns"; public static final String INLINE_PROJECTIONS_ON_VALUES = "inline_projections_on_values"; + public static final String INCLUDE_VALUES_NODE_IN_CONNECTOR_OPTIMIZER = "include_values_node_in_connector_optimizer"; // TODO: Native execution related session properties that are temporarily put here. They will be relocated in the future. public static final String NATIVE_AGGREGATION_SPILL_ALL = "native_aggregation_spill_all"; @@ -1820,6 +1821,10 @@ public SystemSessionProperties( "Whether to evaluate project node on values node", featuresConfig.getInlineProjectionsOnValues(), false), + booleanProperty(INCLUDE_VALUES_NODE_IN_CONNECTOR_OPTIMIZER, + "Include values node for connector optimizer", + featuresConfig.isIncludeValuesNodeInConnectorOptimizer(), + false), integerProperty( NATIVE_MIN_COLUMNAR_ENCODING_CHANNELS_TO_PREFER_ROW_WISE_ENCODING, "Minimum number of columnar encoding channels to consider row wise encoding for partitioned exchange. Native execution only", @@ -3096,6 +3101,11 @@ public static boolean isInlineProjectionsOnValues(Session session) return session.getSystemProperty(INLINE_PROJECTIONS_ON_VALUES, Boolean.class); } + public static boolean isIncludeValuesNodeInConnectorOptimizer(Session session) + { + return session.getSystemProperty(INCLUDE_VALUES_NODE_IN_CONNECTOR_OPTIMIZER, Boolean.class); + } + public static int getMinColumnarEncodingChannelsToPreferRowWiseEncoding(Session session) { return session.getSystemProperty(NATIVE_MIN_COLUMNAR_ENCODING_CHANNELS_TO_PREFER_ROW_WISE_ENCODING, Integer.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 5c2040e6ade8a..94a735c4b1a8b 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 @@ -283,6 +283,7 @@ public class FeaturesConfig private boolean useHistograms; private boolean isInlineProjectionsOnValuesEnabled; + private boolean includeValuesNodeInConnectorOptimizer = true; private boolean eagerPlanValidationEnabled; private int eagerPlanValidationThreadPoolSize = 20; @@ -2810,6 +2811,19 @@ public FeaturesConfig setInlineProjectionsOnValues(boolean isInlineProjectionsOn return this; } + public boolean isIncludeValuesNodeInConnectorOptimizer() + { + return includeValuesNodeInConnectorOptimizer; + } + + @Config("optimizer.include-values-node-in-connector-optimizer") + @ConfigDescription("Include values node in connector optimizer") + public FeaturesConfig setIncludeValuesNodeInConnectorOptimizer(boolean includeValuesNodeInConnectorOptimizer) + { + this.includeValuesNodeInConnectorOptimizer = includeValuesNodeInConnectorOptimizer; + return this; + } + @Config("eager-plan-validation-enabled") @ConfigDescription("Enable eager building and validation of logical plan before queueing") public FeaturesConfig setEagerPlanValidationEnabled(boolean eagerPlanValidationEnabled) diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/ApplyConnectorOptimization.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/ApplyConnectorOptimization.java index 2da87d303b1c2..2fff2446ca791 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/ApplyConnectorOptimization.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/ApplyConnectorOptimization.java @@ -57,11 +57,13 @@ import java.util.Queue; import java.util.Set; +import static com.facebook.presto.SystemSessionProperties.isIncludeValuesNodeInConnectorOptimizer; import static com.facebook.presto.common.RuntimeUnit.NANO; import static com.facebook.presto.sql.OptimizerRuntimeTrackUtil.getOptimizerNameForLog; import static com.facebook.presto.sql.OptimizerRuntimeTrackUtil.trackOptimizerRuntime; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static java.util.Objects.requireNonNull; public class ApplyConnectorOptimization @@ -144,9 +146,9 @@ public PlanOptimizerResult optimize(PlanNode plan, Session session, TypeProvider // * The subtree with root `node` is a closure. // * `node` has no parent, or the subtree with root as `node`'s parent is not a closure. ConnectorPlanNodeContext context = contextMap.get(node); - if (!context.isClosure(connectorId) || + if (!context.isClosure(connectorId, session) || !context.getParent().isPresent() || - contextMap.get(context.getParent().get()).isClosure(connectorId)) { + contextMap.get(context.getParent().get()).isClosure(connectorId, session)) { continue; } @@ -293,10 +295,12 @@ public Set> getReachablePlanNodeTypes() return reachablePlanNodeTypes; } - boolean isClosure(ConnectorId connectorId) + boolean isClosure(ConnectorId connectorId, Session session) { // check if all children can reach the only connector - if (reachableConnectors.size() != 1 || !reachableConnectors.contains(connectorId)) { + boolean includeValuesNode = isIncludeValuesNodeInConnectorOptimizer(session); + Set connectorIds = includeValuesNode ? reachableConnectors.stream().filter(x -> !x.equals(EMPTY_CONNECTOR_ID)).collect(toImmutableSet()) : reachableConnectors; + if (connectorIds.size() != 1 || !connectorIds.contains(connectorId)) { return false; } 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 4ec67782a1ab7..0fe7a44088a97 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 @@ -244,6 +244,7 @@ public void testDefaults() .setRemoveCrossJoinWithSingleConstantRow(true) .setUseHistograms(false) .setInlineProjectionsOnValues(false) + .setIncludeValuesNodeInConnectorOptimizer(true) .setEagerPlanValidationEnabled(false) .setEagerPlanValidationThreadPoolSize(20) .setPrestoSparkExecutionEnvironment(false)); @@ -439,6 +440,7 @@ public void testExplicitPropertyMappings() .put("optimizer.remove-cross-join-with-single-constant-row", "false") .put("optimizer.use-histograms", "true") .put("optimizer.inline-projections-on-values", "true") + .put("optimizer.include-values-node-in-connector-optimizer", "false") .put("eager-plan-validation-enabled", "true") .put("eager-plan-validation-thread-pool-size", "2") .put("presto-spark-execution-environment", "true") @@ -632,6 +634,7 @@ public void testExplicitPropertyMappings() .setRemoveCrossJoinWithSingleConstantRow(false) .setUseHistograms(true) .setInlineProjectionsOnValues(true) + .setIncludeValuesNodeInConnectorOptimizer(false) .setEagerPlanValidationEnabled(true) .setEagerPlanValidationThreadPoolSize(2) .setPrestoSparkExecutionEnvironment(true);