liukun4515 commented on issue #1356: URL: https://github.com/apache/arrow-datafusion/issues/1356#issuecomment-979972001
> > Now, the coercision is in the physical plan or physical expr generation period, but in the spark and other SQL system this action is in the logical period. @alamb > > Yes, this is something @jorgecarleitao wrote -- I think it originally came in as part of [apache/arrow#8024](https://github.com/apache/arrow/pull/8024), which has much more of the rationale. Other systems I have worked on do the coercion at the `LogicalPlan` / `Expr` level, but I think doing coercion at the `ExeuctioPlan` / `PhysicalExpr` level makes sense too. Unless we discover some compelling reason to move coercion to `LogicalPlan` I suggest we do not revisit this decision. > Yes, now we have not meet some issues which should change the coercion to logical period. if we meet them, we can discuss this again. > > I think we can create a new mod for coercion and type inferring. > > The new mod should be pub crate and should not public to out of the datafusion crate. > > This makes sense to me > > > I think we should create diff coercion rule for diff expr type, and we can summary the common logic to the same file. > > Yes I agree > > > In the AggFunction, ScalarFuntion, ScalarUDF, we can define diff coercion to fit each expr and operation. > > I think it might make sense to keep all the coercion logic in the same place (e.g. in the `coercion.rs` module rather than splitting it across `AggFunction`, `ScalarFunction`, etc) but I do not feel super strongly about this I think we have aligned that we can split coercion logic into a independent mod (one sub directory in the `physical_plan` directory). I will try to implement the Agg function to as a sample. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
