Fix the behavior of HAVING clause
#154
Conversation
There is a logic for translating WHERE ... HAVING without GROUP BY to WHERE ... AND, and this bug was causing that every HAVING was rewritten to AND, which resulted in GROUP BY ... HAVING queries like the following one failing: SELECT name, COUNT(*) as count FROM _tmp_table GROUP BY name HAVING COUNT(*) > 1;
The previous method of translating ungrouped HAVING to "AND" had some problems:
1. If no WHERE was in the query, the query would fail with: `near "AND": syntax error`
2. When an aggregate function was used in having (e.g, COUNT(*) > 1), the query
would fail with: `misuse of aggregate function COUNT()`
This commit fixes both of these issues.
JanJakes
force-pushed
the
group-by-having
branch
from
0fdc94c to
3bc55c5
Compare
There was a problem hiding this comment.
@JanJakes, this looks good to me, but I am troubled that our previous test set didn't catch that GROUP BY capture was broken. Before merging this, could we also add a dedicated test for GROUP BY without HAVING?
|
@brandonpayton Thanks for checking this! GROUP BY capture was indeed broken, but not its execution. The capture itself is used only for the HAVING clause. When you use GROUP BY without HAVING, then the value of The way to test this via the public API is GROUP BY with HAVING, and I guess I added some tests for that. |
|
@JanJakes Thanks for the background and explanation. Let's merge this. |
The current implementation has a special handling for
HAVINGclauses withoutGROUP BY(this is valid in MySQL in some SQL modes). This implementation takes advantage of the equivalence of ungrouped HAVING with WHERE and rewrites such aHAVING <condition>clause toAND <condition>.However, the current implementation has some issues:
GROUP BYin a query is incorrectly detected. The detection always fails, and thus every HAVING query is considered to be ungrouped. I added a fix in the first commit.HAVING <condition>toAND <condition>fails with an errornear "AND": syntax errorwhen no WHERE is included in the query. This is fixed in the second commit.COUNT(*) > 1), the query fails with:misuse of aggregate function COUNT(). This is because aggregate functions can't be used in the WHERE clause. This is fixed in the second commit.To fix all of these issues, I implemented a fix for the
GROUP BYdetection (case 1), and changed the rewriting ofHAVING <condition>withoutGROUP BYtoGROUP BY 1 HAVING <condition>(which fixes both case 2 and 3).