gabotechs commented on PR #18739: URL: https://github.com/apache/datafusion/pull/18739#issuecomment-3622446592
> ConfigOptions is already parameter of optimiser method, so whats the reason to extend the method parameter to SessionConfig? From https://github.com/apache/datafusion/pull/18168, which is linked at the beginning of this PR: """ Users might want to create their own PhysicalOptimizerRule implementations. Users might have their own SessionConfig.extensions. Users might want to use their own SessionConfig.extensions in their custom PhysicalOptimizerRule implementations. """ This is the case that we (DataDog) have faced several times already in several of our optimization rules > If you need access to session config extensions, they are arced, they can be introduced using rule constructor as well (rule and extensions are created at the same time). I'd say not necessarily, a rule that is a simple empty struct might be constructed statically. Following this same reasoning, we might also decide that it's better to let `PhysicalOptimizerRule::optimize` take no arguments (not even `ConfigOptions`) as those can also be passed at construction time. > I’m constantly asking what else is needed in OptimiserContext to justify its existence but I haven’t received any answers. As we have a legitimate need for changing `PhysicalOptimizerRule::optimize` to take `SessionConfig` instead of `ConfigOptions` (reason at the beginning of this message), we might as well find more legitimate needs in the future for other new fields, hence we want to commit to API stability by changing the `PhysicalOptimizerRule::optimize` argument only once, as we need to break it now anyways. -- 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]
