[SPARK-35028][SQL] ANSI mode: disallow group by aliases #32129
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gengliangwang
commented
Apr 12, 2021
Member
Author
There was a problem hiding this comment.
Moving ANSI_ENABLED to the front so that other configurations can refer to it without compiling errors.
Member
Author
|
There has been some discussion under the PR that supports group by alias: #17191 Group by aliases is convenient. But it can be ambiguous and incompatible with SQL standard. |
|
Test build #137212 has finished for PR 32129 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
cloud-fan
approved these changes
Apr 12, 2021
Contributor
|
retest this please |
|
Test build #137220 has finished for PR 32129 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
gengliangwang
force-pushed
the
disallowGroupByAlias
branch
from
8c84858 to
62cee4f
Compare
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #137226 has finished for PR 32129 at commit
|
maropu
reviewed
Apr 12, 2021
| The behavior of some SQL operators can be different under ANSI mode (`spark.sql.ansi.enabled=true`). | ||
| - `array_col[index]`: This operator throws `ArrayIndexOutOfBoundsException` if using invalid indices. | ||
| - `map_col[key]`: This operator throws `NoSuchElementException` if key does not exist in map. | ||
| - `GROUP BY`: aliases in a select list can not be used in GROUP BY clauses. Each column referenced in a GROUP BY clause shall unambiguously reference a column of the table resulting from the FROM clause. |
Member
There was a problem hiding this comment.
nit: in a GROUP BY clause -> by a GROUP BY clause?
Member
Author
There was a problem hiding this comment.
Both should work. The second sentence is from the ANSI SQL standard.
maropu
approved these changes
Apr 12, 2021
HyukjinKwon
approved these changes
Apr 13, 2021
Member
|
Nice! |
Member
Author
|
Merging to master |
gengliangwang
added a commit
that referenced
this pull request
Aug 17, 2021
### What changes were proposed in this pull request? Revert [[SPARK-35028][SQL] ANSI mode: disallow group by aliases ](#32129) ### Why are the changes needed? It turns out that many users are using the group by alias feature. Spark has its precedence rule when alias names conflict with column names in Group by clause: always use the table column. This should be reasonable and acceptable. Also, external DBMS such as PostgreSQL and MySQL allow grouping by alias, too. As we are going to announce ANSI mode GA in Spark 3.2, I suggest allowing the group by alias in ANSI mode. ### Does this PR introduce _any_ user-facing change? No, the feature is not released yet. ### How was this patch tested? Unit tests Closes #33758 from gengliangwang/revertGroupByAlias. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>
gengliangwang
added a commit
that referenced
this pull request
Aug 17, 2021
### What changes were proposed in this pull request? Revert [[SPARK-35028][SQL] ANSI mode: disallow group by aliases ](#32129) ### Why are the changes needed? It turns out that many users are using the group by alias feature. Spark has its precedence rule when alias names conflict with column names in Group by clause: always use the table column. This should be reasonable and acceptable. Also, external DBMS such as PostgreSQL and MySQL allow grouping by alias, too. As we are going to announce ANSI mode GA in Spark 3.2, I suggest allowing the group by alias in ANSI mode. ### Does this PR introduce _any_ user-facing change? No, the feature is not released yet. ### How was this patch tested? Unit tests Closes #33758 from gengliangwang/revertGroupByAlias. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]> (cherry picked from commit 8bfb4f1) Signed-off-by: Gengliang Wang <[email protected]>
xuanyuanking
pushed a commit
to xuanyuanking/spark
that referenced
this pull request
Sep 29, 2021
### What changes were proposed in this pull request? Disallow group by aliases under ANSI mode. ### Why are the changes needed? As per the ANSI SQL standard secion 7.12 <group by clause>: >Each `grouping column reference` shall unambiguously reference a column of the table resulting from the `from clause`. A column referenced in a `group by clause` is a grouping column. By forbidding it, we can avoid ambiguous SQL queries like: ``` SELECT col + 1 as col FROM t GROUP BY col ``` ### Does this PR introduce _any_ user-facing change? Yes, group by aliases is not allowed under ANSI mode. ### How was this patch tested? Unit tests Closes apache#32129 from gengliangwang/disallowGroupByAlias. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>
xuanyuanking
pushed a commit
to xuanyuanking/spark
that referenced
this pull request
Sep 29, 2021
### What changes were proposed in this pull request? Revert [[SPARK-35028][SQL] ANSI mode: disallow group by aliases ](apache#32129) ### Why are the changes needed? It turns out that many users are using the group by alias feature. Spark has its precedence rule when alias names conflict with column names in Group by clause: always use the table column. This should be reasonable and acceptable. Also, external DBMS such as PostgreSQL and MySQL allow grouping by alias, too. As we are going to announce ANSI mode GA in Spark 3.2, I suggest allowing the group by alias in ANSI mode. ### Does this PR introduce _any_ user-facing change? No, the feature is not released yet. ### How was this patch tested? Unit tests Closes apache#33758 from gengliangwang/revertGroupByAlias. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]> (cherry picked from commit 8bfb4f1) Signed-off-by: Gengliang Wang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Disallow group by aliases under ANSI mode.
Why are the changes needed?
As per the ANSI SQL standard secion 7.12 :
By forbidding it, we can avoid ambiguous SQL queries like:
Does this PR introduce any user-facing change?
Yes, group by aliases is not allowed under ANSI mode.
How was this patch tested?
Unit tests