Skip to content

fix(parser): Apply HAVING before window functions - #1842

Closed
mbasmanova wants to merge 1 commit into
facebookincubator:mainfrom
mbasmanova:export-D119035291
Closed

fix(parser): Apply HAVING before window functions#1842
mbasmanova wants to merge 1 commit into
facebookincubator:mainfrom
mbasmanova:export-D119035291

Conversation

@mbasmanova

Copy link
Copy Markdown
Contributor

Summary:
A window function nested in a larger expression saw the groups HAVING had already removed, so its aggregate covered rows the query had filtered out:

SELECT a, count(*) / sum(count(*)) OVER (PARTITION BY a)
FROM t GROUP BY a, b HAVING count(*) > 1

The denominator summed every group, not just the ones with more than one row, making each fraction too small. A window that is the whole select item was already ordered correctly; only one nested in an expression was affected.

The HAVING filter is now emitted before the nested-window projection rather than after it, so the window reads the filtered rows.

Differential Revision: D119035291

Summary:
A window function nested in a larger expression saw the groups HAVING had already removed, so its aggregate covered rows the query had filtered out:

    SELECT a, count(*) / sum(count(*)) OVER (PARTITION BY a)
    FROM t GROUP BY a, b HAVING count(*) > 1

The denominator summed every group, not just the ones with more than one row, making each fraction too small. A window that is the whole select item was already ordered correctly; only one nested in an expression was affected.

The HAVING filter is now emitted before the nested-window projection rather than after it, so the window reads the filtered rows.

Differential Revision: D119035291
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 7, 2026
@meta-codesync

meta-codesync Bot commented Sep 7, 2026

Copy link
Copy Markdown

@mbasmanova has exported this pull request. If you are a Meta employee, you can view the originating Diff in D119035291.

@meta-codesync meta-codesync Bot closed this in 6d65c07 Sep 7, 2026
@meta-codesync meta-codesync Bot added the Merged label Sep 7, 2026
@meta-codesync

meta-codesync Bot commented Sep 7, 2026

Copy link
Copy Markdown

This pull request has been merged in 6d65c07.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants