Blizzara commented on code in PR #10653: URL: https://github.com/apache/datafusion/pull/10653#discussion_r1617370361
########## datafusion/substrait/src/logical_plan/producer.rs: ########## @@ -42,7 +42,7 @@ use datafusion::logical_expr::expr::{ }; use datafusion::logical_expr::{expr, Between, JoinConstraint, LogicalPlan, Operator}; use datafusion::prelude::Expr; -use prost_types::Any as ProtoAny; +use pbjson_types::Any as ProtoAny; Review Comment: So this confuses me a bit as well - but the way I understand it, adding the "serde" feature to "substrait-rs" seems to change the types it expects from the original `prost` types into the `pbjson` types (https://github.com/substrait-io/substrait-rs/blob/d11b22c743df9b02b7e5f4c079091ed133982431/build.rs#L262), probably to make them all serde-serializable (https://docs.rs/pbjson-types/latest/pbjson_types/#). Given we use the `Any` proto type in non-test code, I think this does need to change, even though we don't necessarily need the serializability here.. This is also why we need the pbjson-types dep as normal, not dev, dep. I'd also prefer to not change this, but I couldn't find another way if we want to have the JSON proto plans :/ -- 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...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org