milenkovicm commented on code in PR #16986:
URL: https://github.com/apache/datafusion/pull/16986#discussion_r2244458936
##########
datafusion-examples/examples/composed_extension_codec.rs:
##########
@@ -241,14 +253,50 @@ struct ComposedPhysicalExtensionCodec {
}
impl ComposedPhysicalExtensionCodec {
- fn try_any<T>(
+ pub fn new(codecs: Vec<Arc<dyn PhysicalExtensionCodec>>) -> Self {
+ Self { codecs }
+ }
+
+ fn decode_protobuf(
Review Comment:
this function does not really do decoding, it is just trying to find out
which decoder should be used. so name is a bit off.
should se do decoding as well in this function? in the code down we repeat
decoder lookup - decode multiple times.
##########
datafusion-examples/examples/composed_extension_codec.rs:
##########
@@ -233,6 +234,17 @@ impl PhysicalExtensionCodec for
ChildPhysicalExtensionCodec {
}
}
+/// BlobFormatProto captures data encoded by blob format codecs
+#[derive(Clone, PartialEq, prost::Message)]
+struct BlobFormatProto {
+ /// encoder id used to encode blob
+ /// (to be used for decoding)
+ #[prost(uint32, tag = 1)]
+ pub encoder_position: u32,
+ #[prost(bytes, tag = 2)]
+ pub blob: Vec<u8>,
+}
Review Comment:
can we find a bit better name for this structure. Would it make sense to
change doc string to something like: Structure captures information about
encoder used and actual encoded data
##########
datafusion-examples/examples/composed_extension_codec.rs:
##########
@@ -266,26 +314,35 @@ impl PhysicalExtensionCodec for
ComposedPhysicalExtensionCodec {
inputs: &[Arc<dyn ExecutionPlan>],
registry: &dyn FunctionRegistry,
) -> Result<Arc<dyn ExecutionPlan>> {
- self.try_any(|codec| codec.try_decode(buf, inputs, registry))
+ let (codec, blob) = self.decode_protobuf(buf)?;
Review Comment:
we have repeated code in multiple functions should we consider merging those
two functions?
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
