apache / apache/datafusion

Remove `From<String>` for `Column`

Open
#17,375 6 comments 1 reaction 0 assignees View on GitHub
api change bug common
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

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` 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 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`.

Contributor guide

Open the contributing guide

Research direction

Start by locating the listed From<&str>, From<&String>, and From implementations for Column and all usages of those conversions. Review Column::from_qualified_name, replace affected usages with explicit calls, and run the relevant Rust test suite to confirm behavior and compilation.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.