Repository navigation
Integrate Analyzer within LogicalPlan building stage #14618
Description
Is your feature request related to a problem or challenge?
The steps in Logical Layer is Sql->LogicalPlan->Analyzer->Optimizer.
These 5 rules are in Analyzer
Arc::new(InlineTableScan::new()),
// Every rule that will generate [Expr::Wildcard] should be placed in front of [ExpandWildcardRule].
Arc::new(ExpandWildcardRule::new()),
// [Expr::Wildcard] should be expanded before [TypeCoercion]
Arc::new(ResolveGroupingFunction::new()),
Arc::new(TypeCoercion::new()),
Arc::new(CountWildcardRule::new()),The role of the Analyzer is unclear to me. Having two types of "optimization" after the plan is completed doesn’t seem necessary. Instead, we should have one optimization step during plan construction and another after the plan is finalized. I believe these rules can be placed either in the SQL → LogicalPlan building stage or in the optimizer.
Comments of
Analyzer
/// [`AnalyzerRule`]s transform [`LogicalPlan`]s in some way to make
/// the plan valid prior to the rest of the DataFusion optimization process.
///
/// `AnalyzerRule`s are different than an [`OptimizerRule`](crate::OptimizerRule)s
/// which must preserve the semantics of the `LogicalPlan`, while computing
/// results in a more optimal way.
///
/// For example, an `AnalyzerRule` may resolve [`Expr`](datafusion_expr::Expr)s into more specific
/// forms such as a subquery reference, or do type coercion to ensure the types
/// of operands are correct.
If a rule MUST be executed for plan validity (i.e. TypeCoercion), it should be applied during the plan creation stage, not after the plan is completed. However, if the rule is OPTIONAL for plan completion, it should be applied in the optimizer.
I propose removing the concept of the Analyzer and integrating it into the SQL → LogicalPlan stage. Specifically, TypeCoercion should be applied before the plan is finalized (#14380).
Before moving TypeCoercion into the builder, ExpandWildcardRule needs to be relocated first. The remaining three rules can be moved either into the builder or the optimize
Describe the solution you'd like
Requirement
Rules in the Analyzer are optional, allowing users to choose whether to apply them or add custom rules. This flexibility should be preserved, ensuring that the rule remains optional and customizable even after being moved out of the Analyzer.
Tasks
- Move
ExpandWildcardRulein SQL->LogicalPlan stage (MoveExpandWildcardRuleinto Logical Plan construction #14634) - User Defined Coercion Rules #14296 We could make TypeCoercion customizable before moving it out of the Analyzer.
- Move
TypeCoercionin SQL->LogicalPlan stage - Revisit Create UNION plan node with correct schema #14380
- Remove CountWildcardRule in Analyzer and move the functionality in ExprPlanner, add
plan_aggregateandplan_windowto planner #14689 - Investigate InlineTableScan and ResolveGroupingFunction and remove
Analyzerinternally. - Find a way to migrate existing customize analyzer rule away
Describe alternatives you've considered
No response
Additional context
No response
Activity
xudong963 commentedon Feb 12, 2025
+1 for the change
rkrishn7 commentedon Feb 12, 2025
+1
I'd be happy to help here. I can start by taking on moving
ExpandWildcardRuleI opened #14634 to track this
findepi commentedon Feb 12, 2025
I would not call them "optimizations".
They are simply logical ingredients towards building fully resolved Logical Plan (LP).
If we do this issue (+1 from me), we should be able to do
Even without introducing new IR, just making current LP the IR.
So if LP is to be the real IR, we should also plan on making the Expr a real expression:
AliasfromExpr#1468WildcardfromExpr#7765alamb commentedon Feb 15, 2025
Another important consideration I think is that DataFusion is used from non SQL contexts (for example the DataFrame API and when other languages are compiled into LogicalPlans)
The type coercion logic specifically, and likely the other analyzer rules, need to be applied to LogicalPlans that also come from these non SQL sources
findepi commentedon Feb 15, 2025
Good point @alamb . This would mean that the LogicalPlan, being the public API for syntactial query building, is inherently not "fully resolved". Which suggests creating new IR might actually be an easier option that "healing" LP to become fully resolved IR itself.
jayzhan211 commentedon Feb 15, 2025
I agree.
In SQL, the transformation follows this path:
SQL → Expr → LogicalPlan
For DataFrame operations, the transformation is:
Expr → LogicalPlan
The same principles that apply to the Expr → LogicalPlan transformation in the SQL case should also be considered for DataFrame operations. We need to ensure consistency in handling this transformation.
alamb commentedon Feb 15, 2025
A new IR might work
I think @wiedld and I were trying to do the "healing" (well really checking if the
LogicalPlanisn't healthy) via the `InvariantLevel idea here:https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.LogicalPlan.html#method.check_invariants
It isn't yet sidely used, but maybe could be more and more formalized 🤔
jayzhan211 commentedon Mar 24, 2025
I found there is a big issue for moving
TypeCoercionintoLogicalBuilder.We have expression rewrite API like
map_expressionand we can rewrite it after the logical plan is built. In this case having a single final type coercion rule before plan rewrite is a better idea, otherwise we need to bring schema together withExprformap_expressionsso we can recompute the types with schema every time we have a newExpr.#15391 requires breaking change for LogicalBuilder, not sure if it worths another one for
map_expression🤔 . Maybe it is too late to fix this and we might need to find another way to fix #14380jayzhan211 commentedon Mar 24, 2025
In
LogicalPlanBuilder::Values, type coercion is performed before creatingLogicalPlan::Values. This suggests that for cases like Values and Union, having the correct types upfront is necessary, making early type coercion logical. On the other hand, for cases likeExpr::BinaryExpr, it might be more appropriate to defer type coercion until the final stagephillipleblanc commentedon May 21, 2025
@jayzhan211 Is the plan that AnalyzerRules will eventually be deprecated/removed? We rely on a custom AnalyzerRule for SQL federation to remote databases (see datafusion-contrib/datafusion-federation#94). What would be the alternative?