thinkharderdev opened a new issue, #25089:
URL: https://github.com/apache/datafusion/issues/25089
### Is your feature request related to a problem or challenge?
The current `PhysicalExtensionCodec` trait looks like:
```
pub trait PhysicalExtensionCodec: Debug + Send + Sync + Any {
fn try_decode(
&self,
buf: &[u8],
inputs: &[Arc<dyn ExecutionPlan>],
ctx: &TaskContext,
proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<Arc<dyn ExecutionPlan>>;
...other methods
}
```
which is unfortunate because it will cause decoding of extension plans to
fail if they contain scalar subquery expressions. The `ScalarSubqueryResults`
are stages in the `PhysicalPlanDecodeContext` when decoding a
`ScalarSubqueryExec` and are required when decoding a `ScalarSubqueryExpr`
### Describe the solution you'd like
Two options I can see:
1. Breaking change: Change the method signature for
`PhysicalExtensionCodec::try_decode`:
```
pub trait PhysicalExtensionCodec: Debug + Send + Sync + Any {
fn try_decode(
&self,
buf: &[u8],
inputs: &[Arc<dyn ExecutionPlan>],
ctx: PhysicalPlanDecodeContext<'_>,
proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<Arc<dyn ExecutionPlan>>;
...other methods
}
```
Pretty trivial to adapt to but is still a breaking change.
2. Non-breaking change: Add a new method
`PhysicalExtensionCodec:try_decode_with_ctx` which has a default implementation
falling back to the existing method:
```
pub trait PhysicalExtensionCodec: Debug + Send + Sync + Any {
fn try_decode(
&self,
buf: &[u8],
inputs: &[Arc<dyn ExecutionPlan>],
ctx: &TaskContext,
proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<Arc<dyn ExecutionPlan>>;
fn try_decode_with_ctx(
&self,
buf: &[u8],
inputs: &[Arc<dyn ExecutionPlan>],
ctx: PhysicalPlanDecodeContext<'_>,
proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<Arc<dyn ExecutionPlan>> {
self.try_decode(buf, inputs, ctx.task_ctx(), proto_converter)
}
...other methods
}
```
### Describe alternatives you've considered
I *think* you can hack around this with `PhysicalProtoConverterExtension`
somehow but it seems pretty convoluted
### Additional context
_No response_
--
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]
