[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6378: feat: implement serialize/deserialize for extension logical plan

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #6378:
URL: https://github.com/apache/arrow-datafusion/pull/6378#discussion_r1198950905


##########
datafusion/core/src/execution/context.rs:
##########
@@ -2058,6 +2075,33 @@ fn create_dialect_from_str(dialect_name: &str) -> Result<Box<dyn Dialect>> {
     }
 }
 
+/// Deserializer registry for extensions like [UserDefinedLogicalNode].
+pub trait ExtensionDeserializer: Send + Sync {

Review Comment:
   what do you think about putting this trait in `datafusion/execution/src/registry.rs` alongside 
   
   ```
   25: pub trait FunctionRegistry 
   ```
   
   Which serves a similar purpose for user defined functions?



##########
datafusion/expr/src/logical_plan/extension.rs:
##########
@@ -164,6 +164,26 @@ pub trait UserDefinedLogicalNode: fmt::Debug + Send + Sync {
     /// Note: [`UserDefinedLogicalNode`] is not constrained by [`Eq`]
     /// directly because it must remain object safe.
     fn dyn_eq(&self, other: &dyn UserDefinedLogicalNode) -> bool;
+
+    /// Serialize this node to a byte array. This serialization needs not to include

Review Comment:
   I think it might be clearer to say that the serialization *should not* include input plans (as they are serialized by the overall plan serializer, right?)



##########
datafusion/core/src/execution/context.rs:
##########
@@ -2058,6 +2075,33 @@ fn create_dialect_from_str(dialect_name: &str) -> Result<Box<dyn Dialect>> {
     }
 }
 
+/// Deserializer registry for extensions like [UserDefinedLogicalNode].
+pub trait ExtensionDeserializer: Send + Sync {
+    /// Deserialize user defined logical plan node ([UserDefinedLogicalNode]) from
+    /// bytes.
+    fn deserialize_logical_plan(
+        &self,
+        name: &str,
+        bytes: &[u8],
+    ) -> Result<Arc<dyn UserDefinedLogicalNode>>;
+}
+
+/// Default implementation of [ExtensionDeserializer] that throws unimplemented error

Review Comment:
   👍 



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org