Skip to content

Commit

Permalink
Prevent usage of wrong cost calculator before/after AddExchanges
Browse files Browse the repository at this point in the history 
costCalculatorWithEstimatedExchanges should be used before AddExchanges
costCalculatorWithoutEstimatedExchanges should be used after AddExchanges
  • Loading branch information
@sopel39
sopel39 committed Mar 21, 2022
1 parent 70ad1f9 commit b1652b2
Showing 1 changed file with 46 additions and 43 deletions.
89 changes: 46 additions & 43 deletions core/trino-main/src/main/java/io/trino/sql/planner/PlanOptimizers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ public PlanOptimizers(
PageSourceManager pageSourceManager,
StatsCalculator statsCalculator,
ScalarStatsCalculator scalarStatsCalculator,
CostCalculator costCalculator,
@EstimatedExchanges CostCalculator estimatedExchangesCostCalculator,
CostCalculator costCalculatorWithoutEstimatedExchanges,
@EstimatedExchanges CostCalculator costCalculatorWithEstimatedExchanges,
CostComparator costComparator,
TaskCountEstimator taskCountEstimator,
NodePartitioningManager nodePartitioningManager,
Expand All @@ -288,8 +288,8 @@ public PlanOptimizers(
pageSourceManager,
statsCalculator,
scalarStatsCalculator,
costCalculator,
estimatedExchangesCostCalculator,
costCalculatorWithoutEstimatedExchanges,
costCalculatorWithEstimatedExchanges,
costComparator,
taskCountEstimator,
nodePartitioningManager,
Expand All @@ -305,13 +305,15 @@ public PlanOptimizers(
PageSourceManager pageSourceManager,
StatsCalculator statsCalculator,
ScalarStatsCalculator scalarStatsCalculator,
CostCalculator costCalculator,
CostCalculator estimatedExchangesCostCalculator,
CostCalculator costCalculatorWithoutEstimatedExchanges,
CostCalculator costCalculatorWithEstimatedExchanges,
CostComparator costComparator,
TaskCountEstimator taskCountEstimator,
NodePartitioningManager nodePartitioningManager,
RuleStatsRecorder ruleStats)
{
CostCalculator costCalculator = costCalculatorWithEstimatedExchanges;

this.ruleStats = requireNonNull(ruleStats, "ruleStats is null");
ImmutableList.Builder<PlanOptimizer> builder = ImmutableList.builder();

Expand Down Expand Up @@ -348,7 +350,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new InlineProjections(plannerContext, typeAnalyzer),
new RemoveRedundantIdentityProjections()));
Expand All @@ -367,14 +369,14 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
simplifyOptimizerRules);

IterativeOptimizer columnPruningOptimizer = new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
columnPruningRules);

builder.add(
Expand All @@ -383,7 +385,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(new DesugarLambdaExpression().rules())
.addAll(new DesugarAtTimeZone(metadata, typeAnalyzer).rules())
Expand All @@ -397,7 +399,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(new CanonicalizeExpressions(plannerContext, typeAnalyzer).rules())
.add(new OptimizeRowPattern())
Expand All @@ -406,7 +408,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(columnPruningRules)
.addAll(projectionPushdownRules)
Expand Down Expand Up @@ -452,21 +454,21 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new ImplementOffset())),
simplifyOptimizer,
new UnaliasSymbolReferences(metadata),
new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new RemoveRedundantIdentityProjections())),
new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new MergeUnion(),
new MergeIntersect(),
Expand All @@ -476,7 +478,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new ImplementIntersectDistinctAsUnion(metadata),
new ImplementExceptDistinctAsUnion(metadata),
Expand All @@ -489,20 +491,20 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
columnPruningRules),
new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new TransformExistsApplyToCorrelatedJoin(plannerContext))),
new TransformQuantifiedComparisonApplyToCorrelatedJoin(metadata),
new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new RemoveRedundantEnforceSingleRowNode(),
new RemoveUnreferencedScalarSubqueries(),
Expand All @@ -522,7 +524,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new ImplementLimitWithTies(metadata), // must be run after DecorrelateUnnest
new RemoveUnreferencedScalarApplyNodes(),
Expand All @@ -534,7 +536,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new InlineProjections(plannerContext, typeAnalyzer),
new RemoveRedundantIdentityProjections(),
Expand All @@ -551,7 +553,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new RemoveEmptyUnionBranches(),
new EvaluateEmptyIntersect(),
Expand All @@ -572,7 +574,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new ApplyTableScanRedirection(plannerContext),
new PruneTableScanColumns(metadata),
Expand All @@ -594,7 +596,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
pushIntoTableScanRulesExceptJoins);
builder.add(pushIntoTableScanOptimizer);
builder.add(new UnaliasSymbolReferences(metadata));
Expand All @@ -604,7 +606,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(projectionPushdownRules)
.add(new PushProjectionIntoTableScan(plannerContext, typeAnalyzer, scalarStatsCalculator))
Expand All @@ -615,7 +617,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
// Temporary hack: separate optimizer step to avoid the sample node being replaced by filter before pushing
// it to table scan node
ImmutableSet.of(new ImplementBernoulliSampleAsFilter(metadata))),
Expand All @@ -624,7 +626,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new RemoveEmptyUnionBranches(),
new EvaluateEmptyIntersect(),
Expand All @@ -644,7 +646,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(simplifyOptimizerRules) // Should be always run after PredicatePushDown
.add(new PushPredicateIntoTableScan(plannerContext, typeAnalyzer))
Expand All @@ -658,7 +660,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
SystemSessionProperties::useLegacyWindowFilterPushdown,
ImmutableList.of(new WindowFilterPushDown(plannerContext)),
ImmutableSet.of(// should run after DecorrelateUnnest and ImplementLimitWithTies
Expand All @@ -674,7 +676,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
// add UnaliasSymbolReferences when it's ported
.add(new RemoveRedundantIdentityProjections())
Expand All @@ -689,7 +691,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new RemoveRedundantIdentityProjections(),
new PushDownProjectionsFromPatternRecognition())),
Expand All @@ -698,7 +700,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new EliminateCrossJoins(plannerContext, typeAnalyzer))), // This can pull up Filter and Project nodes from between Joins, so we need to push them down again
new StatsRecordingPlanOptimizer(
optimizerStats,
Expand All @@ -707,7 +709,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(simplifyOptimizerRules) // Should be always run after PredicatePushDown
.add(new PushPredicateIntoTableScan(plannerContext, typeAnalyzer))
Expand All @@ -721,7 +723,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(simplifyOptimizerRules) // Should be always run after PredicatePushDown
.add(new PushPredicateIntoTableScan(plannerContext, typeAnalyzer))
Expand All @@ -731,15 +733,15 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new RemoveRedundantIdentityProjections())),
// Prefer write partitioning rule requires accurate stats.
// Run it before reorder joins which also depends on accurate stats.
new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new ApplyPreferredTableWriterPartitioning(),
new ApplyPreferredTableExecutePartitioning())),
Expand All @@ -751,15 +753,15 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new ReorderJoins(plannerContext, costComparator, typeAnalyzer))));

builder.add(new OptimizeMixedDistinctAggregations(metadata));
builder.add(new IterativeOptimizer(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new CreatePartialTopN(),
new PushTopNThroughProject(typeAnalyzer),
Expand Down Expand Up @@ -796,7 +798,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(
new DetermineJoinDistributionType(costComparator, taskCountEstimator), // Must run before AddExchanges
// Must run before AddExchanges and after ReplicateSemiJoinInDelete
Expand All @@ -807,7 +809,7 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(pushIntoTableScanRulesExceptJoins)
// PushJoinIntoTableScan must run after ReorderJoins (and DetermineJoinDistributionType)
Expand All @@ -823,15 +825,16 @@ public PlanOptimizers(
plannerContext,
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
costCalculator,
ImmutableSet.of(new PushTableWriteThroughUnion()))); // Must run before AddExchanges
// unalias symbols before adding exchanges to use same partitioning symbols in joins, aggregations and other
// operators that require node partitioning
builder.add(new UnaliasSymbolReferences(metadata));
builder.add(new StatsRecordingPlanOptimizer(optimizerStats, new AddExchanges(plannerContext, typeAnalyzer, statsCalculator)));
}
//noinspection UnusedAssignment
estimatedExchangesCostCalculator = null; // Prevent accidental use after AddExchanges

// use cost calculator without estimated exchanges after AddExchanges
costCalculator = costCalculatorWithoutEstimatedExchanges;

builder.add(
new IterativeOptimizer(
Expand Down

0 comments on commit b1652b2

Please sign in to comment.