You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "findepi (via GitHub)" <gi...@apache.org> on 2025/09/02 15:19:31 UTC
[I] Remove `From` for `Column` [datafusion]
findepi opened a new issue, #17375:
URL: https://github.com/apache/datafusion/issues/17375
The `From` & `Into` conversion system is a very powerful and convenient feature of Rust.
It should be used when there is only one obvious conversion from source type to the target.
However, this appears not to be the case for some conversion into `Column`.
Consider example
```rust
let field: Field = ....;
Expr::Column(field.name().into()))
```
It seems to create an expression selecting given field.
It is also the simplest conversion from `Field` to `Expr::Column` because, `From<Field>` is not implemented.
However, it doesn't do what it seems it does.
The `From<&str> for Column` invokes name parser and lower-cases the field
These conversions should be removed:
```rust
impl From<&str> for Column {
fn from(c: &str) -> Self {
Self::from_qualified_name(c)
}
}
/// Create a column, cloning the string
impl From<&String> for Column {
fn from(c: &String) -> Self {
Self::from_qualified_name(c)
}
}
/// Create a column, reusing the existing string
impl From<String> for Column {
fn from(c: String) -> Self {
Self::from_qualified_name(c)
}
}
```
Any usages should be replaced with explicit calls to `Column::from_qualified_name`.
--
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-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Remove `From` for `Column` [datafusion]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #17375:
URL: https://github.com/apache/datafusion/issues/17375#issuecomment-3247819239
This is indeed an implicit conversion. May I help fix this issue? @findepi
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Remove `From` for `Column` [datafusion]
Posted by "kartik00052 (via GitHub)" <gi...@apache.org>.
kartik00052 commented on issue #17375:
URL: https://github.com/apache/datafusion/issues/17375#issuecomment-5159054927
hiii @findepi can I work on this issue
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Remove `From` for `Column` [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on issue #17375:
URL: https://github.com/apache/datafusion/issues/17375#issuecomment-3249952422
sure, thanks!
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Remove `From` for `Column` [datafusion]
Posted by "ishan0308 (via GitHub)" <gi...@apache.org>.
ishan0308 commented on issue #17375:
URL: https://github.com/apache/datafusion/issues/17375#issuecomment-3699891481
Hi @findepi
I'm implementing this issue and have a question about public API functions.
After removing `From<String> for Column`, public API functions like `col()`, `out_ref_col()`, and `unnest_column()` can no longer accept strings via `impl Into<Column>` [here](https://github.com/apache/datafusion/blob/main/datafusion/expr/src/expr_fn.rs#L76). I have 2 options
- Change signatures to `impl Into<String>`, internally call `from_qualified_name()` (16 test fixes)
- Keep `impl Into<Column>`, require explicit conversion at ~600 call sites
Which approach is better?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Remove `From` for `Column` [datafusion]
Posted by "ishan0308 (via GitHub)" <gi...@apache.org>.
ishan0308 commented on issue #17375:
URL: https://github.com/apache/datafusion/issues/17375#issuecomment-3695960878
Hi, just checking in on this issue.
Is anyone actively working on it, or can I pick it up?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Remove `From` for `Column` [datafusion]
Posted by "ishanema03 (via GitHub)" <gi...@apache.org>.
ishanema03 commented on issue #17375:
URL: https://github.com/apache/datafusion/issues/17375#issuecomment-3921673344
Hi @findepi
I have raised a PR for this issue
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org