move the type coercion out of the optimizer and refactor the optimizer

[liukun4515]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

When I do this pr https://github.com/apache/arrow-datafusion/pull/3396/files#
I am stuck by many issue about the type in the optimizer framework.

In other SQL or database system, the data type coercion should be done before any optimization.

But we do it in the physical phase, after the @andygrove work and add the TypeCoercion rule in the optimizer. It can be done in the logical phase.

After the https://github.com/apache/arrow-datafusion/pull/3396/files# merge, we need to do the refactor.

If the data type is not right, all the operation and optimization will meet some wired issue.

cc @andygrove @alamb

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

[liukun4515]

I meet the issues, and all of them will be fixed for this issue

• Can't push down projection after do type coercion #3583
• ReduceOuterJoin optimizer support cast or try_cast expr. #3565
• make the header not changed when do optimization #3568
• Support type coercion for subquery #3557
• simplify_expressions don't support different data type for binary #3556
• simplify between expr should consider the data type #3587
• change rule of PreCastLitInComparisonExpressions to unwrap cast rule after #3582 #3622 change the rule
• after type coercion CommonSubexprEliminate will produce invalid projection #3635
• ScalarFunction
  • support type coercion for ScalarFunction expr in the logical phase #3731
  • temporary fix for the contract function: Add type coercion rule for concat and concat_ws #3721
• ScalarUDF remove type coercion for ScalarUDF in the physical phase #3734
• WindowFunction
• AggregateUDF/AggregateFunction move type coercion for agg/agg udf #3752 remove type coercion for agg #3623 AggregateFunction
• move the type coercion out of the optimizer and refactor the optimizer #3582
• support type coercion for case when expr #3673
• Consolidate type coercion: Consolidate coercion code in datafusion_expr::type_coercion and submodules #3728 @alamb

[alamb]

I agree that type coercion doesn't really belong in an "optimizer" as it actually changes the meaning of the exprs (on purpose) where the other optimzier passes are supposed to keep the meaning of the plan the same, but make it faster in some way.

I recommend changing the code to explicitly run the type coercion logic immediately prior to running any other optimization passes. I also think the type coercion logic should not depend on any other simplification (such as constant folding) -- it should depend only on the types.

Once type coercion is done, then we can run the expr simplifier pass to clean up / simplify the expressions

[Dandandan]

I agree - the type coercion should be a mandatory pass that is done after planning.

[liukun4515]

I agree that type coercion doesn't really belong in an "optimizer" as it actually changes the meaning of the exprs (on purpose) where the other optimzier passes are supposed to keep the meaning of the plan the same, but make it faster in some way.

I recommend changing the code to explicitly run the type coercion logic immediately prior to running any other optimization passes. I also think the type coercion logic should not depend on any other simplification (such as constant folding) -- it should depend only on the types.

Once type coercion is done, then we can run the expr simplifier pass to clean up / simplify the expressions

You got it.

[andygrove]

Spark has an Analysis phase that runs before the optimizer. Maybe we can learn from their model. Within the analysis phase, they have a number of type coercion rules that they run:

  override def typeCoercionRules: List[Rule[LogicalPlan]] =
    UnpivotCoercion ::
    WidenSetOperationTypes ::
    new CombinedTypeCoercionRule(
      InConversion ::
      PromoteStrings ::
      DecimalPrecision ::
      BooleanEquality ::
      FunctionArgumentConversion ::
      ConcatCoercion ::
      MapZipWithCoercion ::
      EltCoercion ::
      CaseWhenCoercion ::
      IfCoercion ::
      StackCoercion ::
      Division ::
      IntegralDivision ::
      ImplicitTypeCasts ::
      DateTimeOperations ::
      WindowFrameCoercion ::
      StringLiteralCoercion :: Nil) :: Nil

[liukun4515]

Spark has an Analysis phase that runs before the optimizer. Maybe we can learn from their model. Within the analysis phase, they have a number of type coercion rules that they run:

  override def typeCoercionRules: List[Rule[LogicalPlan]] =
    UnpivotCoercion ::
    WidenSetOperationTypes ::
    new CombinedTypeCoercionRule(
      InConversion ::
      PromoteStrings ::
      DecimalPrecision ::
      BooleanEquality ::
      FunctionArgumentConversion ::
      ConcatCoercion ::
      MapZipWithCoercion ::
      EltCoercion ::
      CaseWhenCoercion ::
      IfCoercion ::
      StackCoercion ::
      Division ::
      IntegralDivision ::
      ImplicitTypeCasts ::
      DateTimeOperations ::
      WindowFrameCoercion ::
      StringLiteralCoercion :: Nil) :: Nil

Yes, I think we can follow other database or sql system.

But It's long way to go and refactor.

My plan is to do some refactor, and make good base for the next big refactor like the spark or other system.

[alamb]

My plan is to do some refactor, and make good base for the next big refactor like the spark or other system.

Sounds like a great plan to me