sgrebnov commented on PR #13915: URL: https://github.com/apache/datafusion/pull/13915#issuecomment-2568661287
@goldmedal - thank you for the feedback. Yeah, other dialects will benefit from similar logic as well. I was exploring how to make this work for all dialects, but it is not possible to fully implement it at the `Dialect` trait level because traits cannot have fields (to store extra overrides). Alternatively: 1. I can add a `with_custom_scalar_overrides` function definition with a blank "not supported yet" implementation at the `Dialect` trait level. This would allow each dialect to implement support as needed (similar to DuckDB) and maintain a unified signature while ensuring that implementation is optional for dialects where it is not currently required. 2. I can implement logic similar to DuckDB for other official dialects so that all dialects support this feature. 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 complexity. @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"); } ... ``` -- 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]
