Skip to content

Commit 197d984

Browse files
tdcmeehantdcmeehan
committed
Migrate evaluateConstantRowExpression to ExpressionOptimizer
This is needed to have consistent constant folding behavior across the codebase.
1 parent 941ffb2 commit 197d984

File tree

7 files changed

+45
-33
lines changed

7 files changed

+45
-33
lines changed

‎presto-main-base/src/main/java/com/facebook/presto/cost/StatsCalculatorModule.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.facebook.presto.cost;
1515

1616
import com.facebook.presto.metadata.Metadata;
17+
import com.facebook.presto.sql.expressions.ExpressionOptimizerManager;
1718
import com.google.common.collect.ImmutableList;
1819
import com.google.inject.Binder;
1920
import com.google.inject.Module;
@@ -46,9 +47,10 @@ public static StatsCalculator createNewStatsCalculator(
4647
StatsNormalizer normalizer,
4748
FilterStatsCalculator filterStatsCalculator,
4849
HistoryBasedPlanStatisticsManager historyBasedPlanStatisticsManager,
49-
FragmentStatsProvider fragmentStatsProvider)
50+
FragmentStatsProvider fragmentStatsProvider,
51+
ExpressionOptimizerManager expressionOptimizerManager)
5052
{
51-
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider);
53+
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider, expressionOptimizerManager);
5254
return historyBasedPlanStatisticsManager.getHistoryBasedPlanStatisticsCalculator(delegate);
5355
}
5456

@@ -57,14 +59,15 @@ public static ComposableStatsCalculator createComposableStatsCalculator(
5759
ScalarStatsCalculator scalarStatsCalculator,
5860
StatsNormalizer normalizer,
5961
FilterStatsCalculator filterStatsCalculator,
60-
FragmentStatsProvider fragmentStatsProvider)
62+
FragmentStatsProvider fragmentStatsProvider,
63+
ExpressionOptimizerManager expressionOptimizerManager)
6164
{
6265
ImmutableList.Builder<ComposableStatsCalculator.Rule<?>> rules = ImmutableList.builder();
6366
rules.add(new OutputStatsRule());
6467
rules.add(new TableScanStatsRule(metadata, normalizer));
6568
rules.add(new SimpleFilterProjectSemiJoinStatsRule(normalizer, filterStatsCalculator, metadata.getFunctionAndTypeManager())); // this must be before FilterStatsRule
6669
rules.add(new FilterStatsRule(normalizer, filterStatsCalculator));
67-
rules.add(new ValuesStatsRule(metadata));
70+
rules.add(new ValuesStatsRule(metadata, expressionOptimizerManager));
6871
rules.add(new LimitStatsRule(normalizer));
6972
rules.add(new EnforceSingleRowStatsRule(normalizer));
7073
rules.add(new ProjectStatsRule(scalarStatsCalculator, normalizer));

‎presto-main-base/src/main/java/com/facebook/presto/cost/ValuesStatsRule.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import com.facebook.presto.matching.Pattern;
2020
import com.facebook.presto.metadata.Metadata;
2121
import com.facebook.presto.spi.plan.ValuesNode;
22+
import com.facebook.presto.spi.relation.ConstantExpression;
2223
import com.facebook.presto.spi.relation.VariableReferenceExpression;
24+
import com.facebook.presto.sql.expressions.ExpressionOptimizerManager;
2325
import com.facebook.presto.sql.planner.TypeProvider;
2426
import com.facebook.presto.sql.planner.iterative.Lookup;
2527

@@ -32,10 +34,12 @@
3234

3335
import static com.facebook.presto.common.type.UnknownType.UNKNOWN;
3436
import static com.facebook.presto.cost.StatsUtil.toStatsRepresentation;
37+
import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.EVALUATED;
3538
import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel.FACT;
36-
import static com.facebook.presto.sql.planner.RowExpressionInterpreter.evaluateConstantRowExpression;
3739
import static com.facebook.presto.sql.planner.plan.Patterns.values;
40+
import static com.google.common.base.Verify.verify;
3841
import static com.google.common.collect.ImmutableList.toImmutableList;
42+
import static java.util.Objects.requireNonNull;
3943
import static java.util.stream.Collectors.toList;
4044

4145
public class ValuesStatsRule
@@ -44,10 +48,12 @@ public class ValuesStatsRule
4448
private static final Pattern<ValuesNode> PATTERN = values();
4549

4650
private final Metadata metadata;
51+
private final ExpressionOptimizerManager expressionOptimizerManager;
4752

48-
public ValuesStatsRule(Metadata metadata)
53+
public ValuesStatsRule(Metadata metadata, ExpressionOptimizerManager expressionOptimizerManager)
4954
{
50-
this.metadata = metadata;
55+
this.metadata = requireNonNull(metadata, "metadata is null");
56+
this.expressionOptimizerManager = requireNonNull(expressionOptimizerManager, "expressionOptimizerManager is null");
5157
}
5258

5359
@Override
@@ -82,7 +88,11 @@ private List<Object> getVariableValues(ValuesNode valuesNode, int symbolId, Sess
8288
}
8389
return valuesNode.getRows().stream()
8490
.map(row -> row.get(symbolId))
85-
.map(rowExpression -> evaluateConstantRowExpression(rowExpression, metadata.getFunctionAndTypeManager(), session.toConnectorSession()))
91+
.map(rowExpression -> expressionOptimizerManager.getExpressionOptimizer(session.toConnectorSession())
92+
.optimize(rowExpression, EVALUATED, session.toConnectorSession(), i -> i))
93+
.peek(rowExpression -> verify(rowExpression instanceof ConstantExpression, "Expected constant expression, but got: %s", rowExpression))
94+
.map(rowExpression -> (ConstantExpression) rowExpression)
95+
.map(ConstantExpression::getValue)
8696
.collect(toList());
8797
}
8898

‎presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ public PlanOptimizers(
744744
builder.add(predicatePushDown); // Run predicate push down one more time in case we can leverage new information from layouts' effective predicate
745745
builder.add(simplifyRowExpressionOptimizer); // Should be always run after PredicatePushDown
746746

747-
builder.add(new MetadataQueryOptimizer(metadata));
747+
builder.add(new MetadataQueryOptimizer(metadata, expressionOptimizerManager));
748748

749749
// This can pull up Filter and Project nodes from between Joins, so we need to push them down again
750750
builder.add(

‎presto-main-base/src/main/java/com/facebook/presto/sql/planner/RowExpressionInterpreter.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@
109109
import static com.google.common.base.Preconditions.checkArgument;
110110
import static com.google.common.base.Preconditions.checkState;
111111
import static com.google.common.base.Predicates.instanceOf;
112-
import static com.google.common.base.Verify.verify;
113112
import static com.google.common.collect.ImmutableList.toImmutableList;
114113
import static com.google.common.collect.Iterables.getOnlyElement;
115114
import static io.airlift.slice.Slices.utf8Slice;
@@ -130,15 +129,6 @@ public class RowExpressionInterpreter
130129
private final FunctionResolution resolution;
131130

132131
private final Visitor visitor;
133-
134-
public static Object evaluateConstantRowExpression(RowExpression expression, FunctionAndTypeManager functionAndTypeManager, ConnectorSession session)
135-
{
136-
// evaluate the expression
137-
Object result = new RowExpressionInterpreter(expression, functionAndTypeManager, session, EVALUATED).evaluate();
138-
verify(!(result instanceof RowExpression), "RowExpression interpreter returned an unresolved expression");
139-
return result;
140-
}
141-
142132
public static RowExpressionInterpreter rowExpressionInterpreter(RowExpression expression, FunctionAndTypeManager functionAndTypeManager, ConnectorSession session)
143133
{
144134
return new RowExpressionInterpreter(expression, functionAndTypeManager, session, EVALUATED);

‎presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/MetadataQueryOptimizer.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import com.facebook.presto.spi.relation.RowExpression;
4747
import com.facebook.presto.spi.relation.VariableReferenceExpression;
4848
import com.facebook.presto.spi.statistics.TableStatistics;
49+
import com.facebook.presto.sql.expressions.ExpressionOptimizerManager;
4950
import com.facebook.presto.sql.planner.TypeProvider;
5051
import com.facebook.presto.sql.planner.VariablesExtractor;
5152
import com.facebook.presto.sql.planner.plan.SimplePlanRewriter;
@@ -64,9 +65,10 @@
6465
import java.util.Set;
6566

6667
import static com.facebook.presto.spi.plan.ProjectNode.Locality.LOCAL;
67-
import static com.facebook.presto.sql.planner.RowExpressionInterpreter.evaluateConstantRowExpression;
68+
import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.EVALUATED;
6869
import static com.facebook.presto.sql.relational.Expressions.call;
6970
import static com.facebook.presto.sql.relational.Expressions.constant;
71+
import static com.google.common.base.Verify.verify;
7072
import static com.google.common.collect.ImmutableList.toImmutableList;
7173
import static com.google.common.collect.Iterables.getOnlyElement;
7274
import static java.util.Objects.requireNonNull;
@@ -81,12 +83,13 @@ public class MetadataQueryOptimizer
8183
private final Set<QualifiedObjectName> allowedFunctions;
8284
private final Map<QualifiedObjectName, QualifiedObjectName> aggregationScalarMapping;
8385
private final Metadata metadata;
86+
private final ExpressionOptimizerManager expressionOptimizerManager;
8487

85-
public MetadataQueryOptimizer(Metadata metadata)
88+
public MetadataQueryOptimizer(Metadata metadata, ExpressionOptimizerManager expressionOptimizerManager)
8689
{
87-
requireNonNull(metadata, "metadata is null");
90+
this.metadata = requireNonNull(metadata, "metadata is null");
91+
this.expressionOptimizerManager = requireNonNull(expressionOptimizerManager, "expressionOptimizerManager is null");
8892

89-
this.metadata = metadata;
9093
CatalogSchemaName defaultNamespace = metadata.getFunctionAndTypeManager().getDefaultNamespace();
9194
this.allowedFunctions = ImmutableSet.of(
9295
QualifiedObjectName.valueOf(defaultNamespace, "max"),
@@ -104,7 +107,7 @@ public PlanOptimizerResult optimize(PlanNode plan, Session session, TypeProvider
104107
if (!SystemSessionProperties.isOptimizeMetadataQueries(session) && !SystemSessionProperties.isOptimizeMetadataQueriesIgnoreStats(session)) {
105108
return PlanOptimizerResult.optimizerResult(plan, false);
106109
}
107-
Optimizer optimizer = new Optimizer(session, metadata, idAllocator);
110+
Optimizer optimizer = new Optimizer(session, metadata, idAllocator, expressionOptimizerManager);
108111
PlanNode rewrittenPlan = SimplePlanRewriter.rewriteWith(optimizer, plan, null);
109112
return PlanOptimizerResult.optimizerResult(rewrittenPlan, optimizer.isPlanChanged());
110113
}
@@ -130,16 +133,18 @@ private static class Optimizer
130133
private final int metastoreCallNumThreshold;
131134
private boolean planChanged;
132135
private final MetadataQueryOptimizer metadataQueryOptimizer;
136+
private final ExpressionOptimizerManager expressionOptimizerManager;
133137

134-
private Optimizer(Session session, Metadata metadata, PlanNodeIdAllocator idAllocator)
138+
private Optimizer(Session session, Metadata metadata, PlanNodeIdAllocator idAllocator, ExpressionOptimizerManager expressionOptimizerManager)
135139
{
136140
this.session = session;
137141
this.metadata = metadata;
138142
this.idAllocator = idAllocator;
139143
this.determinismEvaluator = new RowExpressionDeterminismEvaluator(metadata);
140144
this.ignoreMetadataStats = SystemSessionProperties.isOptimizeMetadataQueriesIgnoreStats(session);
141145
this.metastoreCallNumThreshold = SystemSessionProperties.getOptimizeMetadataQueriesCallThreshold(session);
142-
this.metadataQueryOptimizer = new MetadataQueryOptimizer(metadata);
146+
this.metadataQueryOptimizer = new MetadataQueryOptimizer(metadata, expressionOptimizerManager);
147+
this.expressionOptimizerManager = expressionOptimizerManager;
143148
}
144149

145150
public boolean isPlanChanged()
@@ -374,15 +379,17 @@ private RowExpression evaluateMinMax(FunctionMetadata aggregationFunctionMetadat
374379
List<RowExpression> reducedArguments = new ArrayList<>();
375380
// We fold for every 100 values because GREATEST/LEAST has argument count limit
376381
for (List<RowExpression> partitionedArguments : Lists.partition(arguments, 100)) {
377-
Object reducedValue = evaluateConstantRowExpression(
382+
RowExpression expression = expressionOptimizerManager.getExpressionOptimizer(connectorSession).optimize(
378383
call(
379384
metadata.getFunctionAndTypeManager(),
380385
scalarFunctionName,
381386
returnType,
382387
partitionedArguments),
383-
metadata.getFunctionAndTypeManager(),
384-
connectorSession);
385-
reducedArguments.add(constant(reducedValue, returnType));
388+
EVALUATED,
389+
connectorSession,
390+
i -> i);
391+
verify(expression instanceof ConstantExpression, "Expected constant expression");
392+
reducedArguments.add(expression);
386393
}
387394
arguments = reducedArguments;
388395
}

‎presto-main-base/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ private LocalQueryRunner(Session defaultSession, FeaturesConfig featuresConfig,
478478
this.filterStatsCalculator = new FilterStatsCalculator(metadata, scalarStatsCalculator, statsNormalizer);
479479
this.historyBasedPlanStatisticsManager = new HistoryBasedPlanStatisticsManager(objectMapper, createTestingSessionPropertyManager(), metadata, new HistoryBasedOptimizationConfig(), featuresConfig, new NodeVersion("1"));
480480
this.fragmentStatsProvider = new FragmentStatsProvider();
481-
this.statsCalculator = createNewStatsCalculator(metadata, scalarStatsCalculator, statsNormalizer, filterStatsCalculator, historyBasedPlanStatisticsManager, fragmentStatsProvider);
481+
this.statsCalculator = createNewStatsCalculator(metadata, scalarStatsCalculator, statsNormalizer, filterStatsCalculator, historyBasedPlanStatisticsManager, fragmentStatsProvider, expressionOptimizerManager);
482482
this.taskCountEstimator = new TaskCountEstimator(() -> nodeCountForStats);
483483
this.costCalculator = new CostCalculatorUsingExchanges(taskCountEstimator);
484484
this.estimatedExchangesCostCalculator = new CostCalculatorWithEstimatedExchanges(costCalculator, taskCountEstimator);

‎presto-spark-base/src/main/java/com/facebook/presto/spark/planner/PrestoSparkStatsCalculatorModule.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.facebook.presto.cost.StatsCalculator;
2323
import com.facebook.presto.cost.StatsNormalizer;
2424
import com.facebook.presto.metadata.Metadata;
25+
import com.facebook.presto.sql.expressions.ExpressionOptimizerManager;
2526
import com.google.inject.Binder;
2627
import com.google.inject.Module;
2728
import com.google.inject.Provides;
@@ -55,9 +56,10 @@ public static StatsCalculator createNewStatsCalculator(
5556
FilterStatsCalculator filterStatsCalculator,
5657
FragmentStatsProvider fragmentStatsProvider,
5758
HistoryBasedPlanStatisticsManager historyBasedPlanStatisticsManager,
58-
HistoryBasedOptimizationConfig historyBasedOptimizationConfig)
59+
HistoryBasedOptimizationConfig historyBasedOptimizationConfig,
60+
ExpressionOptimizerManager expressionOptimizerManager)
5961
{
60-
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider);
62+
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider, expressionOptimizerManager);
6163
return new PrestoSparkStatsCalculator(historyBasedPlanStatisticsManager.getHistoryBasedPlanStatisticsCalculator(delegate), delegate, historyBasedOptimizationConfig);
6264
}
6365
}

0 commit comments

Comments
 (0)