mihaibudiu commented on code in PR #4417:
URL: https://github.com/apache/calcite/pull/4417#discussion_r2157687894
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6589,12 +6610,14 @@ public boolean isBangEqualAllowed() {
sql("select deptno + empno as d, deptno + empno + mgr from emp"
+ " group by d,mgr")
.withConformance(lenient).ok();
- // When alias is equal to one or more columns in the query then giving
- // priority to alias. But Postgres may throw ambiguous column error or give
- // priority to column name.
+ // Alias is not used since there is a single column in the SELECT already
matching
sql("select count(*) from (\n"
- + " select ename AS deptno FROM emp GROUP BY deptno) t")
- .withConformance(lenient).ok();
+ + " select ^ename^ AS deptno FROM emp GROUP BY deptno) t")
+ .withConformance(strict)
+ .fails("Expression 'ENAME' is not being grouped")
+ .withConformance(lenient)
Review Comment:
@xiedeyantu I would like to better understand your concerns. This stuff is
very subtle, and it would be nice to get it right.
There are 3 features touched in this PR:
1. alias resolution in GROUP BY
2. alias resolution in HAVING
3. alias resolution in SELECT (lateral column aliasing) (I should have not
changed this at all in this PR, but read further)
As far as I know, the current implementation behaves like MySQL with respect
to alias resolution in 1. GROUP BY and 2. HAVING. It also behaves like Spark
with respect to alias resolution in 3. SELECT. MySQL does not seem to have 3,
lateral column aliasing.
This implementation may be compatible with Spark with respect to 1. GROUP BY
and 2. HAVING too, but I don't have an easy way to check. Neither of these
systems has a spec of the exact behavior.
For MySQL I have validated all tests in aliasing.iq; they give the same
results. I don't know if the tests cover all combinations possible; I tried to
cover all combinations of features 1 & 2.
Unfortunately these three features are not independent, and they interact in
subtle ways. I don't have a good way to test whether our current implementation
is "sound", since MySQL supports only 1 & 2 of the features.
Do you find the comments in the code incorrect or unclear?
There is one major principle that should be obeyed by any implementation:
the semantics of any legal query in "standard" SQL (without these features)
does not change after these features are implemented. The bug I am fixing with
this PR is exactly in this space: legal queries became illegal, and some legal
queries produced different results.
How illegal queries in standard SQL behave - there you have choices. Some
will remain illegal, some will become legal. I tried to emulate MySQL as far as
I could test, since I expect it is not unsound.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
