goldmedal commented on PR #13915: URL: https://github.com/apache/datafusion/pull/13915#issuecomment-2572217309
> Regarding adding overrides at the `Unparser` level: I like that this approach would work for all dialects. However, my concern is that there are cases where configuration is done solely at the `Dialect` level without access to the `Unparser`. For example, when using [datafusion-federation](https://github.com/datafusion-contrib/datafusion-federation/blob/d3626ce72f723d4c715d2012e72cbac6b4fca67d/datafusion-federation/src/sql/mod.rs#L673). Additionally, splitting configuration between two entities—`Unparser` and `Dialect`—where both control SQL overrides during unparsing, could introduce extra complexity/conflicts. > Thanks for the information. Indeed, the custom behavior should depend on the dialect instead of the unparser. > @goldmedal - WDYT if we proceed with approach 1 above, where we add the `with_custom_scalar_overrides` function definition at the `Dialect` trait level and add implementation based on needs? > > ```rust > pub trait Dialect: Send + Sync { > ... > fn with_custom_scalar_overrides( > self, > _handlers: Vec<(&str, ScalarFnToSqlHandler)>, > ) -> Self > where > Self: Sized, > { > unimplemented!("Dialect does not support defining custom scalar overrides"); > } > ... > ``` It makes sense to me 👍. The user can use this method in `scalar_function_to_sql_overrides` if required. -- 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]
