alamb commented on PR #3885: URL: https://github.com/apache/arrow-datafusion/pull/3885#issuecomment-1289628826
> I'm really struggling to review this PR as I'm honestly lost trying to understand the design of the config system... Yes, I agree it is very confusing. Would you find it less confusing if the physical planner got a `SessionConfig` rather than `ConfigOptions`? > Plumbing this down into the physical plans therefore feels a bit like it is coupling together parts of the system that shouldn't be coupled together? I would have expected the physical operators to only be exposed to TaskContext, with SessionContext solely used for planning. I think the `SessionContext` has a `SessionState` which has a `SessionConfig` which has a `ConfigOptions` 🤯 TaskContext has `TaskProperties` which is either a `SessionConfig` or some k=v pairs So in that view of things your expectation "expected the physical operators to only be exposed to TaskContext, with SessionContext solely used for planning." does hold In my mind `ConfigOptions` and `SessionConfig` serve the same purpose and eventually they will be unified (using the implementation of `ConfigOptions`. -- 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]
