-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Is your feature request related to a problem or challenge?
With #16970 SessionContext has been propagated as parameter of most methods in physical plan proto apart from PhysicalExtensionCodec::try_decode which still uses registry: &dyn FunctionRegistry,. This is a bit of a problem when try_decode calls other methods in physical plan proto as there is no ctx to provide as parameter
Describe the solution you'd like
I would argue that SessionContext is overkill and that all methods should align on TaskContext as a parameter a it holds all required information.
Describe alternatives you've considered
Changing PhysicalExtensionCodec::try_decode registry to SessionContext has been considered, but I would argue SessionContext is to wide interface to be used here
Additional context
This issue has arisen when updating ballista to datafusion 50. A part of the issue is that decoder has been invoked on executor side where only TaskContext can be provided
relevant ballista pr apache/datafusion-ballista#1320