vbarua commented on code in PR #12225:
URL: https://github.com/apache/datafusion/pull/12225#discussion_r1746023670
##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -439,6 +458,27 @@ pub async fn from_substrait_rel(
}
names.insert(new_name);
}
+ let schema = input.schema();
+ if let (Some(extensions_names), true) =
+ (extensions.names.as_ref(), p.common.is_some())
Review Comment:
Using the final output names here doesn't really make sense. Those are
intended to apply names to the results of the Substrait plan.
Given a Substrait plan like:
```
Project[a + b]
NamedScan[a, b, c, d, e]
```
What the DataFusion Project needs to include is all of the fields of the
input relation like:
```
DataFusionProject[a, b, c, d, e, a + b]
DataFusionTable[a, b, c, d, e]
```
##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -214,6 +214,9 @@ pub async fn from_substrait_plan(
Ok(from_substrait_rel(ctx, rel, &extensions).await?)
},
plan_rel::RelType::Root(root) => {
+ if !root.names.is_empty() {
+ extensions =
extensions.with_projection_names(root.names.clone());
+ }
Review Comment:
What's the point of storing the output names in the extension like this?
They shouldn't be needed anywhere else but here, and it looks like the code
below already applies them to the generated plan.
--
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
