You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "blaginin (via GitHub)" <gi...@apache.org> on 2025/03/24 18:58:21 UTC
[I] Scalars are too verbose [datafusion]
blaginin opened a new issue, #15395:
URL: https://github.com/apache/datafusion/issues/15395
### Is your feature request related to a problem or challenge?
When parsing scalar expressions, DataFusion makes queries quite complicated:
```sql
> select 3, array_length([1, 2, 4, 5, 10000, 2.4]);;
+----------+-----------------------------------------------------------------------------------------+
| Int64(3) | array_length(make_array(Int64(1),Int64(2),Int64(4),Int64(5),Int64(10000),Float64(2.4))) |
+----------+-----------------------------------------------------------------------------------------+
| 3 | 6 |
+----------+-----------------------------------------------------------------------------------------+
```
Such detailed comparison isn't needed in most cases and adds unnecessary cognitive complexity for developers. Moreover, it also breaks the original query - if you put `select Int64(3)`, you'll get `Error during planning: Invalid function 'int64'`
### Describe the solution you'd like
In most cases, the simplest version will be enough. On the example above it is:
```sql
> select 3, array_length([1, 2, 4, 5, 10000, 2.4]);
+---+---------------------------------------------+
| 3 | array_length(make_array(1,2,4,5,10000,2.4)) |
+---+---------------------------------------------+
| 3 | 6 |
+---+---------------------------------------------+
```
IMO there are three ways how to fix this problem:
1. Just remove `:?` here for all cases
https://github.com/apache/datafusion/blob/0ff89844a2f2c7a3bbcaff995db7e464deaeb997/datafusion/expr/src/expr.rs#L2951
This will make queries simpler everywhere. The downside is that we lose some information, e.g., type info in the shell (which one can argue isn't needed and can be checked via other types)
2. Disable verbose as a ConfigOptions param and make it changeable (via datafusion-cli or sdk).
3. Use short names only if parsing back preserves the correct type. For example, when formatting `Int64(0)`, output `0` (since parsing `0` results in `Int64(0)`). When formatting `Int32(0)`, keep `Int32(0)`.
### Describe alternatives you've considered
_No response_
### Additional context
May be quite easy to update after we finish https://github.com/apache/datafusion/issues/15178
--
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] Scalars are too verbose in column name output [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2755188180
> I think @irenjj added similar code recently to support new duckdb style tree explain plans via Expr::sql_name
We can potentially use `human_display` instead of `schema_name` when creating schemas. Though tbf I really like the idea of always rendering data in a more compact way (the first option from my list) - it should be even easier to implement ๐ค
--
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] Scalars are too verbose [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2749654314
If we have column type, we don't need to display type for inner elements. Maybe we can work on column type first?
--
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] Scalars are too verbose [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2749411350
for context duckdb:
```
D select 3, array_length([1, 2, 4, 5, 10000, 2.4]);
โโโโโโโโโฌโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โ 3 โ array_length(main.list_value(1, 2, 4, 5, 10000, 2.4)) โ
โ int32 โ int64 โ
โโโโโโโโโผโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโค
โ 3 โ 6 โ
โโโโโโโโโดโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
```
--
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] Scalars are too verbose in column name output [datafusion]
Posted by "irenjj (via GitHub)" <gi...@apache.org>.
irenjj commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2753138770
> Here is a related ticket:
>
> * [beautify default column namesย #2027](https://github.com/apache/datafusion/issues/2027)
>
> > Created an issue to ask what others think - the change is quite simple, but maybe there were previous discussions or consequences I don't see?
>
> I think [@irenjj](https://github.com/irenjj) added similar code recently to support new duckdb style `tree` explain plans via `Expr::sql_name`:
>
> * [Simplify display format of `AggregateFunctionExpr`, add `Expr::sql_name`ย #15253](https://github.com/apache/datafusion/pull/15253)
>
> However, it seems like the display is still somewhat ๐คฎ (I think b/c the column name is bad)
>
> > explain format tree select 3, array_length([1, 2, 4, 5, 10000, 2.4]);;
> +---------------+-------------------------------+
> | plan_type | plan |
> +---------------+-------------------------------+
> | physical_plan | โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ |
> | | โ ProjectionExec โ |
> | | โ -------------------- โ |
> | | โ Int64(3): 3 โ |
> | | โ โ |
> | | โ array_length(make_array โ |
> | | โ (Int64(1),Int64(2) โ |
> | | โ ,Int64(4),Int64(5) โ |
> | | โ ,Int64(10000),Float64 โ |
> | | โ (2.4))): โ |
> | | โ 6 โ |
> | | โโโโโโโโโโโโโโโฌโโโโโโโโโโโโโโ |
> | | โโโโโโโโโโโโโโโดโโโโโโโโโโโโโโ |
> | | โ PlaceholderRowExec โ |
> | | โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ |
> | | |
> +---------------+-------------------------------+
> 1 row(s) fetched.
> Elapsed 0.007 seconds.
> Perhaps we should switch to `tree` by default as well as use `sql_name` for the column name ๐ค
>
> * [[DISCUSS] Switch to `tree` explain by defaultย #15343](https://github.com/apache/datafusion/issues/15343)
`sql_name` only works in aggr func, need more effort for projection, window, ...
--
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] Scalars are too verbose [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2749417038
Thank you Bruce!!! Btw adding _column type_ (second line of the first row) can actually be a good thing to implement separately
--
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] Scalars are too verbose in column name output [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2931763906
I feel like with https://github.com/apache/datafusion/issues/15178 almost done, this ticket can finally be wrapped up โบ๏ธ
> The challenge is that it will change the schema of the output
That's true. For example above, the column name would change from `Int64(3)` to `3`. It's definitely a breaking change, so maybe we should roll it into the next major release?
I've been thinking about what could break for users, and came up with:
- Expected column name. But the case with raw ints feels pretty minor, especially if you exclude people who donโt give the column a meaningful name (which I think is the intended behaviour).
- Column duplicates, e.g. we had `Int64(1)` and `Int32(1)` but theyโd both now become just `1`. This feels like a very weird edge case. And moreover hard to achieve: if you used `arrow_cast` to set the type, it'll still be in the column name.
- Harder to understand the column type - but I think https://github.com/apache/datafusion/issues/15442 already addresses that.
--
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] Scalars are too verbose in column name output [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2752511719
Here is a related ticket:
- #2027
> Created an issue to ask what others think - the change is quite simple, but maybe there were previous discussions or consequences I don't see?
I think @irenjj added similar code recently to support new duckdb style `tree` explain plans via `Expr::sql_name`:
- https://github.com/apache/datafusion/pull/15253
However, it seems like the display is still somewhat ๐คฎ (I think b/c the column name is bad)
```sql
> explain format tree select 3, array_length([1, 2, 4, 5, 10000, 2.4]);;
+---------------+-------------------------------+
| plan_type | plan |
+---------------+-------------------------------+
| physical_plan | โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ |
| | โ ProjectionExec โ |
| | โ -------------------- โ |
| | โ Int64(3): 3 โ |
| | โ โ |
| | โ array_length(make_array โ |
| | โ (Int64(1),Int64(2) โ |
| | โ ,Int64(4),Int64(5) โ |
| | โ ,Int64(10000),Float64 โ |
| | โ (2.4))): โ |
| | โ 6 โ |
| | โโโโโโโโโโโโโโโฌโโโโโโโโโโโโโโ |
| | โโโโโโโโโโโโโโโดโโโโโโโโโโโโโโ |
| | โ PlaceholderRowExec โ |
| | โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ |
| | |
+---------------+-------------------------------+
1 row(s) fetched.
Elapsed 0.007 seconds.
```
Perhaps we should switch to `tree` by default as well as use `sql_name` for the column name ๐ค
- https://github.com/apache/datafusion/issues/15343
--
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] Scalars are too verbose in column name output [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2755667376
> always rendering data in a more compact way (the first option from my list) -
I think it is a better choice too
The challenge is that it will change the schema of the output (the output column name will change) as will the internal column names
--
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] Scalars are too verbose in column name output [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2755180609
> If we have column type, we don't need to display type for inner elements. Maybe we can work on column type first?
Thank you!!! That's fair, I've created a separate ticket: https://github.com/apache/datafusion/issues/15442
--
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] Scalars are too verbose [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on issue #15395:
URL: https://github.com/apache/datafusion/issues/15395#issuecomment-2749127690
Created an issue to ask what others think - the change is quite simple, but maybe there were previous discussions or consequences I don't see?
--
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