kosiew commented on code in PR #22917:
URL: https://github.com/apache/datafusion/pull/22917#discussion_r3426213859
##########
datafusion/sql/src/planner.rs:
##########
@@ -575,7 +575,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let fields = plan.schema().fields().clone();
LogicalPlanBuilder::from(plan)
.project(fields.iter().zip(idents).map(|(field, ident)| {
-
col(field.name()).alias(self.ident_normalizer.normalize(ident))
+
Expr::Column(Column::from_qualified_name_ignore_case(field.name()))
Review Comment:
`apply_expr_alias` still rebuilds a column reference by reparsing
`field.name()` as SQL text. That helps with the simple case-sensitive name
case, but quoted identifiers can legally contain separators like `.`.
For example, `CREATE TABLE t ("A.B" int); SELECT * FROM (SELECT * FROM t)
t_(x);` is still resolved as a qualified column `A.B` instead of the existing
field named `A.B`, so aliasing still errors.
I think the important invariant here is positional: each existing output
field should be aliased while preserving its already resolved schema identity.
Could we build `Expr::Column` from `plan.schema().columns()` or
`qualified_field` instead of parsing the display name string? One way to do
that would be to collect the schema columns before moving `plan`, then zip
those with `idents`.
--
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]
