adriangb commented on code in PR #26824:
URL: https://github.com/apache/datafusion/pull/26824#discussion_r4395211465


##########
datafusion/datasource/src/source.rs:
##########
@@ -304,14 +304,52 @@ pub trait DataSource: Any + Send + Sync + Debug {
     ///
     /// [`PhysicalPlanNode`]: 
datafusion_proto_models::protobuf::PhysicalPlanNode
     #[cfg(feature = "proto")]
+    #[deprecated(since = "56.0.0", note = "implement `try_to_proto_source` 
instead")]
+    ///
+    /// The default wraps [`Self::try_to_proto_source`], so callers that still
+    /// forward to this hook on a migrated source get the layered node.
     fn try_to_proto(
         &self,
-        _ctx: &datafusion_physical_plan::proto::ExecutionPlanEncodeCtx<'_>,
+        ctx: &datafusion_physical_plan::proto::ExecutionPlanEncodeCtx<'_>,
     ) -> Result<Option<datafusion_proto_models::protobuf::PhysicalPlanNode>> {
+        Ok(self.try_to_proto_source(ctx)?.map(data_source_plan_node))

Review Comment:
   The deprecated hooks are unchanged again: their defaults return `None` as in 
55, and the built-ins implement only the new hooks. So no deprecated encoder 
writes the layered form, and the deprecated decoders still read the flat form 
they always read. The upgrade guide tells wrapper authors to forward the new 
hook. (da361cbd9)
   



-- 
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]

Reply via email to