vbarua commented on code in PR #23672:
URL: https://github.com/apache/datafusion/pull/23672#discussion_r3617353593
##########
datafusion/substrait/src/logical_plan/producer/rel/read_rel.rs:
##########
@@ -173,25 +138,16 @@ pub fn from_empty_relation(
.iter()
.map(|f| {
let scalar = ScalarValue::try_from(f.data_type())?;
- to_substrait_literal(producer, &scalar)
+ producer.handle_expr(&Expr::Literal(scalar, None),
&empty_schema)
})
.collect::<datafusion::common::Result<_>>()?;
ReadType::VirtualTable(VirtualTable {
- // Use deprecated 'values' field instead of 'expressions' because
the consumer's
- // nested expression support (RexType::Nested) is not yet
implemented.
- // The 'values' field uses literal::Struct which the consumer can
properly
- // deserialize with field name preservation.
- #[expect(deprecated)]
- values: vec![LiteralStruct { fields }],
- expressions: vec![],
Review Comment:
Something I'm realizing, for migration purpose we may first want to ship a
change that updates this to _write_ these values using expressions.
If we start off right of the bat removing all support for values, anyone
storing plans outside of DataFusion won't be able to read them post update,
because we'll only handle expressions.
--
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]
