[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3970: fix: check recursion limit in `Expr::to_bytes`

[GitBox <gi...@apache.org>]

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


##########
datafusion/proto/src/bytes/mod.rs:
##########
@@ -84,7 +87,45 @@ impl Serializeable for Expr {
             DataFusionError::Plan(format!("Error encoding protobuf as bytes: {}", e))
         })?;
 
-        Ok(buffer.into())
+        let bytes: Bytes = buffer.into();
+
+        // the produced byte stream may lead to "recursion limit" errors, see
+        // https://github.com/apache/arrow-datafusion/issues/3968
+        // Until the underlying prost issue ( https://github.com/tokio-rs/prost/issues/736 ) is fixed, we try to
+        // deserialize the data here and check for errors.
+        //
+        // Need to provide some placeholder registry because the stream may contain UDFs
+        struct PlaceHolderRegistry;
+
+        impl FunctionRegistry for PlaceHolderRegistry {

Review Comment:
   I think the comments explain what is going on 👍 



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