andygrove commented on code in PR #22069:
URL: https://github.com/apache/datafusion/pull/22069#discussion_r3204737439
##########
datafusion/expr/src/utils.rs:
##########
@@ -339,7 +339,38 @@ fn get_excluded_columns(
idents.push(&excepts.first_element);
idents.extend(&excepts.additional_elements);
}
+ // Declared outside the `if let` so `idents.extend(exclude_owned.iter())`
+ // below can borrow references that outlive the inner scope.
+ #[cfg(feature = "sql")]
+ let exclude_owned: Vec<sqlparser::ast::Ident>;
if let Some(exclude) = opt_exclude {
+ #[cfg(feature = "sql")]
Review Comment:
Claude says:
Here's what's happening and why the cfg(feature = "sql") gates exist:
The setup. datafusion-expr has a sql feature (on by default) that depends
on sqlparser. When sql is enabled, ExcludeSelectItem is re-exported from
sqlparser::ast. When it's disabled, expr.rs provides its
own fallback enum with Ident payloads
(datafusion/expr/src/expr.rs:1431-1436). The two paths in utils.rs already
exist via the existing #[cfg(...)] imports at lines 41-45.
What sqlparser 0.62 changed. In 0.61, ExcludeSelectItem::Single carried
Ident and Multiple carried Vec<Ident>. In 0.62, both now carry ObjectName (a
possibly multi-part name like schema.column). The
fallback enum in expr.rs still uses Ident because that crate-local copy
hasn't changed.
--
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]
