alamb commented on code in PR #13294:
URL: https://github.com/apache/datafusion/pull/13294#discussion_r1833138506
##########
datafusion/sql/src/planner.rs:
##########
@@ -401,6 +401,14 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
pub(crate) fn convert_data_type(&self, sql_type: &SQLDataType) ->
Result<DataType> {
+ // First check if any of the registered expr_planners can handle this
type
+ for expr_planner in self.context_provider.get_expr_planners() {
Review Comment:
It seems strange that the planner for individual expressions would handle
query-wide data types (though I realize this is currently the only hook that is
available into the sql planner)
What would you think about adding a `DataTypePlanner` trait / field to the
planner explicitly for this conversion?
##########
datafusion/expr/src/planner.rs:
##########
@@ -205,6 +206,13 @@ pub trait ExprPlanner: Debug + Send + Sync {
fn plan_any(&self, expr: RawBinaryExpr) ->
Result<PlannerResult<RawBinaryExpr>> {
Ok(PlannerResult::Original(expr))
}
+
+ /// Plan SQL type to DataFusion data type
+ ///
+ /// Returns None if not possible
+ fn plan_data_type(&self, _sql_type: &ast::DataType) ->
Result<Option<DataType>> {
Review Comment:
👍
--
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]
