milenkovicm commented on code in PR #18813:
URL: https://github.com/apache/datafusion/pull/18813#discussion_r2541165308
##########
datafusion-examples/examples/composed_extension_codec.rs:
##########
@@ -52,26 +53,30 @@ async fn main() {
let exec_plan = Arc::new(ParentExec {
input: Arc::new(ChildExec {}),
});
- let ctx = SessionContext::new();
+ let task_ctx = SessionContext::new().task_ctx();
// Position in this list is important as it will be used for decoding.
// If new codec is added it should go to last position.
- let composed_codec = ComposedPhysicalExtensionCodec::new(vec![
+ let composed_codec = Arc::new(ComposedPhysicalExtensionCodec::new(vec![
Arc::new(ParentPhysicalExtensionCodec {}),
Arc::new(ChildPhysicalExtensionCodec {}),
- ]);
+ ]));
+ let mut parser = TaskContextWithPhysicalCodec {
Review Comment:
I find `parser` as a name may be a bit confusing
##########
datafusion/proto/src/physical_plan/to_proto.rs:
##########
@@ -50,20 +50,20 @@ use crate::protobuf::{
PhysicalSortExprNodeCollection,
};
-use super::PhysicalExtensionCodec;
+use super::PhysicalSerializer;
#[expect(clippy::needless_pass_by_value)]
-pub fn serialize_physical_aggr_expr(
+pub fn serialize_physical_aggr_expr<S: PhysicalSerializer>(
aggr_expr: Arc<AggregateFunctionExpr>,
- codec: &dyn PhysicalExtensionCodec,
+ parser: &mut S,
Review Comment:
I find name parser a bit confusing, but i have no better suggestion at the
moment, maybe `serializer`?
##########
datafusion/ffi/src/udaf/accumulator_args.rs:
##########
@@ -63,11 +63,11 @@ impl TryFrom<AccumulatorArgs<'_>> for FFI_AccumulatorArgs {
WrappedSchema(FFI_ArrowSchema::try_from(args.return_field.as_ref())?);
let schema = WrappedSchema(FFI_ArrowSchema::try_from(args.schema)?);
- let codec = DefaultPhysicalExtensionCodec {};
+ let mut codec = DefaultPhysicalExtensionCodec {};
Review Comment:
Is there a plan @timsaucer to make this configurable so we can use our own
codec? It’s likely covered in other PRs, is it?
##########
datafusion/proto/src/physical_plan/from_proto.rs:
##########
@@ -73,14 +72,13 @@ impl From<&protobuf::PhysicalColumn> for Column {
/// * `input_schema` - The Arrow schema for the input, used for determining
expression data types
/// when performing type coercion.
/// * `codec` - An extension codec used to decode custom UDFs.
-pub fn parse_physical_sort_expr(
+pub fn parse_physical_sort_expr<D: PhysicalDeserializer>(
proto: &protobuf::PhysicalSortExprNode,
- ctx: &TaskContext,
+ parser: &mut D,
Review Comment:
`deserializer` rather than `parser` ?
--
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]
