jdye64 commented on code in PR #4892:
URL: https://github.com/apache/arrow-datafusion/pull/4892#discussion_r1069645995


##########
datafusion/substrait/src/serializer.rs:
##########
@@ -27,15 +27,19 @@ use std::fs::OpenOptions;
 use std::io::{Read, Write};
 
 pub async fn serialize(sql: &str, ctx: &SessionContext, path: &str) -> 
Result<()> {
+    let protobuf_out = serialize_bytes(sql, ctx);
+    let mut file = OpenOptions::new().create(true).write(true).open(path)?;
+    file.write_all(&protobuf_out)?;
+    Ok(())
+}
+
+pub async fn serialize_bytes(sql: &str, ctx: &SessionContext) -> Vec::<u8> {
     let df = ctx.sql(sql).await?;
     let plan = df.into_optimized_plan()?;
     let proto = producer::to_substrait_plan(&plan)?;
 
     let mut protobuf_out = Vec::<u8>::new();
-    proto.encode(&mut protobuf_out).unwrap();
-    let mut file = OpenOptions::new().create(true).write(true).open(path)?;
-    file.write_all(&protobuf_out)?;
-    Ok(())
+    let result = proto.encode(&mut protobuf_out).unwrap();

Review Comment:
   You are correct that shouldn't compile. I foolishly just ran `cargo build` 
in the Datafusion root directory and when the build succeeded assumed all was 
well. Taking a closer look I see that `datafusion/substrait` is not part of 
that base `Cargo.toml` file so this was in fact never built and why I didn't 
see the error.
   
   I will make the syntax changes now for this to compile now.
   
   I was going to add `datafusion/substrait` to the root `Cargo.toml` but noted 
issue https://github.com/apache/arrow-datafusion/pull/4893 so probably need to 
wait until that is resolved first otherwise windows builds would all fail.



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

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

Reply via email to