Skip to content

Commit 88ba01c

Browse files
tdcmeehantdcmeehan
committed
Migrate evaluateConstantRowExpression to ExpressionOptimizer
This is needed to have consistent constant folding behavior across the codebase.
1 parent 02ee89f commit 88ba01c

File tree

7 files changed

+44
-33
lines changed

7 files changed

+44
-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;
@@ -45,9 +46,10 @@ public static StatsCalculator createNewStatsCalculator(
4546
StatsNormalizer normalizer,
4647
FilterStatsCalculator filterStatsCalculator,
4748
HistoryBasedPlanStatisticsManager historyBasedPlanStatisticsManager,
48-
FragmentStatsProvider fragmentStatsProvider)
49+
FragmentStatsProvider fragmentStatsProvider,
50+
ExpressionOptimizerManager expressionOptimizerManager)
4951
{
50-
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider);
52+
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider, expressionOptimizerManager);
5153
return historyBasedPlanStatisticsManager.getHistoryBasedPlanStatisticsCalculator(delegate);
5254
}
5355

@@ -56,14 +58,15 @@ public static ComposableStatsCalculator createComposableStatsCalculator(
5658
ScalarStatsCalculator scalarStatsCalculator,
5759
StatsNormalizer normalizer,
5860
FilterStatsCalculator filterStatsCalculator,
59-
FragmentStatsProvider fragmentStatsProvider)
61+
FragmentStatsProvider fragmentStatsProvider,
62+
ExpressionOptimizerManager expressionOptimizerManager)
6063
{
6164
ImmutableList.Builder<ComposableStatsCalculator.Rule<?>> rules = ImmutableList.builder();
6265
rules.add(new OutputStatsRule());
6366
rules.add(new TableScanStatsRule(metadata, normalizer));
6467
rules.add(new SimpleFilterProjectSemiJoinStatsRule(normalizer, filterStatsCalculator, metadata.getFunctionAndTypeManager())); // this must be before FilterStatsRule
6568
rules.add(new FilterStatsRule(normalizer, filterStatsCalculator));
66-
rules.add(new ValuesStatsRule(metadata));
69+
rules.add(new ValuesStatsRule(metadata, expressionOptimizerManager));
6770
rules.add(new LimitStatsRule(normalizer));
6871
rules.add(new EnforceSingleRowStatsRule(normalizer));
6972
rules.add(new ProjectStatsRule(scalarStatsCalculator, normalizer));

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

Lines changed: 13 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,10 @@ 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).getValue())
8695
.collect(toList());
8796
}
8897

‎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
@@ -766,7 +766,7 @@ public PlanOptimizers(
766766
builder.add(predicatePushDown); // Run predicate push down one more time in case we can leverage new information from layouts' effective predicate
767767
builder.add(simplifyRowExpressionOptimizer); // Should be always run after PredicatePushDown
768768

769-
builder.add(new MetadataQueryOptimizer(metadata));
769+
builder.add(new MetadataQueryOptimizer(metadata, expressionOptimizerManager));
770770

771771
// This can pull up Filter and Project nodes from between Joins, so we need to push them down again
772772
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
@@ -113,7 +113,6 @@
113113
import static com.google.common.base.Preconditions.checkArgument;
114114
import static com.google.common.base.Preconditions.checkState;
115115
import static com.google.common.base.Predicates.instanceOf;
116-
import static com.google.common.base.Verify.verify;
117116
import static com.google.common.collect.ImmutableList.toImmutableList;
118117
import static com.google.common.collect.Iterables.getOnlyElement;
119118
import static io.airlift.slice.Slices.utf8Slice;
@@ -133,15 +132,6 @@ public class RowExpressionInterpreter
133132
private final FunctionResolution resolution;
134133

135134
private final Visitor visitor;
136-
137-
public static Object evaluateConstantRowExpression(RowExpression expression, FunctionAndTypeManager functionAndTypeManager, ConnectorSession session)
138-
{
139-
// evaluate the expression
140-
Object result = new RowExpressionInterpreter(expression, functionAndTypeManager, session, EVALUATED).evaluate();
141-
verify(!(result instanceof RowExpression), "RowExpression interpreter returned an unresolved expression");
142-
return result;
143-
}
144-
145135
public static RowExpressionInterpreter rowExpressionInterpreter(RowExpression expression, FunctionAndTypeManager functionAndTypeManager, ConnectorSession session)
146136
{
147137
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
@@ -473,7 +473,7 @@ private LocalQueryRunner(Session defaultSession, FeaturesConfig featuresConfig,
473473
this.filterStatsCalculator = new FilterStatsCalculator(metadata, scalarStatsCalculator, statsNormalizer);
474474
this.historyBasedPlanStatisticsManager = new HistoryBasedPlanStatisticsManager(objectMapper, createTestingSessionPropertyManager(), metadata, new HistoryBasedOptimizationConfig(), featuresConfig, new NodeVersion("1"));
475475
this.fragmentStatsProvider = new FragmentStatsProvider();
476-
this.statsCalculator = createNewStatsCalculator(metadata, scalarStatsCalculator, statsNormalizer, filterStatsCalculator, historyBasedPlanStatisticsManager, fragmentStatsProvider);
476+
this.statsCalculator = createNewStatsCalculator(metadata, scalarStatsCalculator, statsNormalizer, filterStatsCalculator, historyBasedPlanStatisticsManager, fragmentStatsProvider, expressionOptimizerManager);
477477
this.taskCountEstimator = new TaskCountEstimator(() -> nodeCountForStats);
478478
this.costCalculator = new CostCalculatorUsingExchanges(taskCountEstimator);
479479
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;
@@ -54,9 +55,10 @@ public static StatsCalculator createNewStatsCalculator(
5455
FilterStatsCalculator filterStatsCalculator,
5556
FragmentStatsProvider fragmentStatsProvider,
5657
HistoryBasedPlanStatisticsManager historyBasedPlanStatisticsManager,
57-
HistoryBasedOptimizationConfig historyBasedOptimizationConfig)
58+
HistoryBasedOptimizationConfig historyBasedOptimizationConfig,
59+
ExpressionOptimizerManager expressionOptimizerManager)
5860
{
59-
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider);
61+
StatsCalculator delegate = createComposableStatsCalculator(metadata, scalarStatsCalculator, normalizer, filterStatsCalculator, fragmentStatsProvider, expressionOptimizerManager);
6062
return new PrestoSparkStatsCalculator(historyBasedPlanStatisticsManager.getHistoryBasedPlanStatisticsCalculator(delegate), delegate, historyBasedOptimizationConfig);
6163
}
6264
}

0 commit comments

Comments
 (0)