dqkqd commented on PR #18286: URL: https://github.com/apache/datafusion/pull/18286#issuecomment-3507856286
@Jefffrey As you mentioned, the solution for `LogicalPlan::Values` isn't intuitive and quite hard to understand. I reverted the changes and only fix for `LogicalPlan::EmptyRelation` in this PR. Some contexts: I tried these, but couldn't find a good solution: 1. Not using `recompute_schema`, add a projection layer to cast the datatype (using `coerce_plan_expr_for_schema`). This was wrong, the physical plan cannot be executed because of data type mismatch. 2. Adding a new method `LogicalPlanBuilder::values_with_coerce_type(values, schema)` (from this [comment](https://github.com/apache/datafusion/pull/18286#discussion_r2493427812)) and use it in `recompute_schema`. I found it hard to explain, and I wasn't sure whether coercing data types could fully answer this TODO: https://github.com/apache/datafusion/blob/6ab4d216b768c9327982e59376a62a29c69ca436/datafusion/expr/src/logical_plan/plan.rs#L636-L637 3. Explicitly handling `LogicalPlan::Values`, extract the plan's schema, change it data types after replacing param values. This was safe but I thought it quite verbose. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
