sainad2222 commented on code in PR #24973:
URL: https://github.com/apache/datafusion/pull/24973#discussion_r4075065352
##########
datafusion/proto/src/physical_plan/mod.rs:
##########
@@ -2093,6 +2334,56 @@ impl ComposedPhysicalExtensionCodec {
.encode(buf)
.map_err(|e| internal_datafusion_err!("{e}"))
}
+
+ /// Like [`Self::encode_protobuf`], but for the function hooks whose trait
+ /// default is `Ok(())` rather than an error.
+ ///
+ /// Those hooks treat an empty buffer as "no custom payload, encode by
+ /// name", and the decode side only consults the function registry when the
+ /// payload is absent. Wrapping an empty blob in a [`DataEncoderTuple`]
+ /// would make a by-name function look codec-encoded and strand it at
+ /// decode time, so a codec that writes nothing must leave `buf` untouched.
+ fn encode_protobuf_by_name_aware(
+ &self,
+ buf: &mut Vec<u8>,
+ mut encode: impl FnMut(&dyn PhysicalExtensionCodec, &mut Vec<u8>) ->
Result<()>,
+ ) -> Result<()> {
Review Comment:
On agent's idea: `decode_protobuf` dispatches on `encoder_position`, so an
empty buf always hits codec 0 and errors before reaching position 1. By-name
functions have no payload anyway, so the registry resolves them before any
codec is asked.
Going ahead with your idea
--
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]
