thinkharderdev commented on code in PR #9395:
URL: https://github.com/apache/arrow-datafusion/pull/9395#discussion_r1509371775


##########
datafusion/proto/tests/cases/roundtrip_logical_plan.rs:
##########
@@ -631,6 +638,12 @@ pub mod proto {
         #[prost(uint64, tag = "1")]
         pub k: u64,
     }
+
+    #[derive(Clone, PartialEq, ::prost::Message)]
+    pub struct ScalarUDFProto {
+        #[prost(message, tag = "1")]
+        pub expr: 
::core::option::Option<datafusion_proto::protobuf::LogicalExprNode>,
+    }

Review Comment:
   This is not how this would work. Expressions would be passed to the udf as 
arguments. What would potentially get serialized into `ScalarUDFProto` would 
just be some non-expr state. For example, a regex UDF might look like:
   
   ```
   struct MyRegexUdf {
     // compiled regex
     regex: Regex,
     // regex as original string
     pattern: String,
   }
   
   impl ScalarUdfImpl for MyRegexUdf {
    // implementation here, not important for example
   }
   ```
   
   This would take a `Utf8` expression in as an argument when evaluated but 
that will be serialized and deserialized separately. In the proto we only need 
to serialized the regex. So the proto would be something like
   
   ```
   message MyRegexUdfNode {
     string pattern = 1;
   }
   ```
   
   Then in our custom codec we have
   
   ```
   fn try_decode_udf(&self, name: &str, buf: &[u8]) -> Result<Arc<ScalarUDF>> {
       if let Ok(proto) = MyRegexUdfNode::decode(buf) {
         let regex = Regex::new(&proto.pattern).map_err(|err| ...)?;
         Ok(Arc::new(
           ScalarUDF::new_from_impl(MyRegexUdf {
              regex,
              pattern: proto.pattern,
            })
         ))
       } else {
         not_impl_err!("unrecognized scalar UDF implementation, cannot decode")
       }
   }
   ```
   
   



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

Reply via email to