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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
