Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect plan for implicit GROUP BY with HAVING #733

Merged
merged 1 commit into from May 9, 2019

Conversation

4 participants
@martint
Copy link
Member

commented May 9, 2019

Per the SQL spec,

  1) Let HC be the <having clause>. Let TE be the <table expression> that immediately
     contains HC. If TE does not immediately contain a <group by clause>, then “GROUP BY ()”
     is implicit.

@cla-bot cla-bot bot added the cla-signed label May 9, 2019

Fix incorrect plan for implicit GROUP BY with HAVING
Per the SQL spec,

  1) Let HC be the <having clause>. Let TE be the <table expression> that immediately
     contains HC. If TE does not immediately contain a <group by clause>, then “GROUP BY ()”
     is implicit.

@martint martint force-pushed the martint:having branch from da38746 to a078a27 May 9, 2019

@dain

dain approved these changes May 9, 2019

@martint martint merged commit 04a4850 into prestosql:master May 9, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
verification/cla-signed
Details

@martint martint referenced this pull request May 9, 2019

Closed

Release notes for 311 #716

5 of 5 tasks complete
.assertQuery(
"SELECT 'x' FROM (VALUES 1, 1, 2) t(a) HAVING true",
"VALUES 'x'");
}

This comment has been minimized.

Copy link
@findepi

findepi May 10, 2019

Member

This commit (intentionally) changes behavior. Previously:

presto> select a from (values 1,2,3) t(a) having a > 1;
 a
---
 2
 3

now the query fails with

line 1:8: 'a' must be an aggregate expression or appear in GROUP BY clause

Would be good to have an explicit test for this.

@martint martint added the correctness label May 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.