You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/18 15:18:57 UTC
[GitHub] [arrow-datafusion] comphead opened a new pull request, #3882: simplify unqualified col name aliases
comphead opened a new pull request, #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #3722 .
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
Use simple names in schema - use the function name in this example and do not add the parameters.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Use simple names in schema - use the function name in this example and do not add the parameters.
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
The unqualified column name will be changed
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#discussion_r999898918
##########
datafusion/core/tests/sql/select.rs:
##########
@@ -505,14 +505,14 @@ async fn use_between_expression_in_select_query() -> Result<()> {
let actual = execute_to_batches(&ctx, sql).await;
// Expect field name to be correctly converted for expr, low and high.
let expected = vec![
- "+-------------------------------------------------------------+",
- "| abs(test.c1) BETWEEN Int64(0) AND log(test.c1 * Int64(100)) |",
- "+-------------------------------------------------------------+",
- "| true |",
- "| true |",
- "| false |",
- "| false |",
- "+-------------------------------------------------------------+",
+ "+------------------------------+",
Review Comment:
The plan changes sure look nicer 😍
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] comphead commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
comphead commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1282996135
@andygrove please check this PR
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1293608254
Marking this as a draft as it is waiting on feedback and not ready for review or merge
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1287230665
> @alamb @Dandandan should I put unique col aliasing like trim_0, trim_1 as part of this PR, or create a new one?
I am not sure -- perhaps @andygrove can weigh in here as he filed https://github.com/apache/arrow-datafusion/issues/3722
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
andygrove commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1293777462
Postgres allows duplicate column names in a schema but then throws an error if you try and select one of those columns. We currently fail on creating a schema with duplicate columns.
I guess the first question is, do we want to be more like Postgres here?
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] comphead commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
comphead commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1284577878
> What happens when using the same function more than once?
@Dandandan thanks for you comment, the engine historically fails if the alias are the same, like if you run
`let sql = "SELECT COALESCE(NULL, 'test'), COALESCE(NULL, 'test')";`
it will fail even now.
supposingly we need a separate ticket to try to do postgres behaviuor, or add some number like
`coalesce_0, coalesce_1`. I'm happy to do this if needed
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Dandandan commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
Dandandan commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1284843513
Ok, but the current branch now also fails with
`select trim(a), trim(b) from table`? As the two are now giving the same alias. This is my main worry to merge this. As suffix that @comphead suggests seems a solution.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] simplify unqualified col name aliases [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1830643392
Closing as this PR is over a year old. Please feel free to reopen it / rebase it if you plan to keep working on it
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] comphead commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
comphead commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1284895937
> Ok, but the current branch now also fails with `select trim(a), trim(b) from table`? As the two are now giving the same alias. This is my main worry to merge this. As suffix that @comphead suggests seems a solution.
This is correct, as we shortened the column name the collision will happen more often. But this gives us more motivation to make unique readable aliases.
Because currently user may not be happy to refer to column from outer sql to nested subquery which contains not friendly alias like `btrim(test.a,Utf8(\"ab\"))`
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
andygrove commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1291283223
I will catch up on this and other pending PRs tomorrow.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] comphead commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
comphead commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1285700140
@alamb @Dandandan should I put unique col aliasing like `trim_0, trim_1` as part of this PR, or create a new one?
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] simplify unqualified col name aliases [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed pull request #3882: simplify unqualified col name aliases
URL: https://github.com/apache/arrow-datafusion/pull/3882
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1285490386
BTW the way postgres handles this, as I recall, is to refer to each output created by a node using its index the output schema rather than a string
I don't recommend going this way as debugging errors with mixed up output indexes was both painful and common
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] comphead commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
comphead commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1293980339
> Postgres allows duplicate column names in a schema but then throws an error if you try and select one of those columns. We currently fail on creating a schema with duplicate columns.
>
> I guess the first question is, do we want to be more like Postgres here?
Thanks @andygrove for the feedback.
Most likely we should avoid current naming for not qualified columns as its not user friendly and hard to use in outer subqueries.
And perhaps its a bigger topic to discuss naming standard, to be like a Postgres or something else, but we can create a discussion ticket on this first, to get all options from contributors.
If you ask me, I like how its done in Trino
`
select t.*, 'col_again' from (
select 'col', 2 a, count(1)
) t
`
```
_col0|a|_col2|_col3 |
-----+-+-----+---------+
col |2| 1|col_again|
```
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Dandandan commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
Dandandan commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1284535147
Not sure I'm a fan. What happens with the alias if you use a function on more than one column?
E.g. `select trim(a), trim(b) from table`?
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1284561242
Here is what postgres does:
```
postgres=# select * from foo;
val | time
-----+------
A | 1.0
B | 2.0
C | 3.0
(3 rows)
postgres=# select trim(val), trim(val) from foo;
btrim | btrim
-------+-------
A | A
B | B
C | C
(3 rows)
postgres=#
```
Here is what datafusion-cli does from master:
```
DataFusion CLI v13.0.0
❯ create table foo as select column1 as val, column2 as time from (values ('A', 1.0), ('B', 2.0), ('C', 3.0)) as sq;
0 rows in set. Query took 0.023 seconds.
❯ select trim(val), trim(val) from foo;
Plan("Projections require unique expression names but the expression \"trim(foo.val)\" at position 0 and \"trim(foo.val)\" at position 1 have the same name. Consider aliasing (\"AS\") one of them.")
```
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] comphead commented on pull request #3882: simplify unqualified col name aliases
Posted by GitBox <gi...@apache.org>.
comphead commented on PR #3882:
URL: https://github.com/apache/arrow-datafusion/pull/3882#issuecomment-1290791946
> > @alamb @Dandandan should I put unique col aliasing like trim_0, trim_1 as part of this PR, or create a new one?
>
> I am not sure -- perhaps @andygrove can weigh in here as he filed #3722
@andygrove please help here until the code rots
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org