Standardize the separator in name
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Is your feature request related to a problem or challenge?
While working on replacing GetFieldAccess with `get_field` function, I got the [name checking error](https://github.com/apache/datafusion/blob/b21bf9e8527de82e901c3a61127d63779f230163/datafusion/physical-expr/src/equivalence/projection.rs#L70-L73) that due to the mismatch of separator in creating name. I found that most of the name built with single comma and space, so I think we should switch others to this format `.join(", ")`.
Error is like
```
External error: query failed: DataFusion error: Internal error: Input field name SUM(get_field(CASE WHEN get_field(t2.struct(t1.time,t1.load1,t1.load2,t1.host), Utf8("c3")) IS NOT NULL THEN t2.struct(t1.time,t1.load1,t1.load2,t1.host) END,Utf8("c2"))) does not match with the projection expression SUM(get_field(CASE WHEN get_field(t2.struct(t1.time,t1.load1,t1.load2,t1.host),Utf8("c3")) IS NOT NULL THEN t2.struct(t1.time,t1.load1,t1.load2,t1.host) END,Utf8("c2"))).
This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker
[SQL] select t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] as host, sum((case when t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] is not null then t2."struct(t1.time,t1.load1,t1.load2,t1.host)" end)['c2']) from (select struct(time,load1,load2,host) from t1) t2 where t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] IS NOT NULL group by t2."struct(t1.time,t1.load1,t1.load2,t1.host)"['c3'] order by host;
```
I got one with `t2.struct(t1.time,t1.load1,t1.load2,t1.host), Utf8("c3")` and another `t2.struct(t1.time,t1.load1,t1.load2,t1.host),Utf8("c3")`. That is why I think we should fix the separator.
Since the change is quite large, I would like to wait for more comment.
### Describe the solution you'd like
_No response_
### Describe alternatives you've considered
_No response_
### Additional context
_No response_
You can find the error [here](https://github.com/jayzhan211/arrow-datafusion/commit/41c871d530cef177bf5784e09f48dd0f1044e6ab) with `cargo test --test sqllogictests -- expr`. Although, it is not on the main branch, but I think most of the changes are unrelated to the name checking issue.
Contributor guide
Research direction
Start with datafusion/physical-expr/src/equivalence/projection.rs at the linked name-checking code, then use the reported cargo test --test sqllogictests -- expr command to reproduce the mismatch. Trace the name construction involved in the example and verify that the affected names consistently use ", " as their separator and that the relevant SQL logic tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100