milenkovicm commented on PR #18739: URL: https://github.com/apache/datafusion/pull/18739#issuecomment-3622289668
I don't think you understand me correctly, `SessionConfig` has two properties, `ConfigOptions` and `extensions`. `ConfigOptions` is already parameter of optimiser method, so whats the reason to extend the method parameter to `SessionConfig`? If you need something else than `ConfigOptions` introduce it as part of the your rule constructor, you can make reference to your rule or your rule property, or whatever you want. 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’m constantly asking what else is needed in `OptimiserContext` to justify its existence but I haven’t received any answers. Also note, you have marked old method for removal (Btw, there is difference, between logical optimiser and physical, logical accepts trait not a struct, which makes difference) My point isn’t whether it’s easy to update after a change. Writing code is easy these days but we should maintain a much higher standard for ourselves than just writing a code. -- 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]
