fix: Use of HAVING outside of GROUP BY should raise a suitable SQLSyntaxError

[alexander-beedie]

Ref #19310.

While some (but not all) SQL dialects allow a bare HAVING (eg: outside of GROUP BY context) it is usually clearer to implement such constraints using a WHERE. However, currently we are silently ignoring such HAVING clauses :\

We should raise a syntax error here for now (can consider allowing such clauses in the future, though use of HAVING outside of GROUP BY is pretty niche - it requires implicitly treating all rows as a single group, and therefore for the SELECT to be composed of aggregate functions, but in the meantime can be written with a follow-on WHERE clause instead).

Example

import polars as pl

df = pl.DataFrame({
    "key": ["aaa", "aaa", "bbb", "bbb", "bbb"],
    "value": [100, 25, -200, 50, 135],
})

Bare HAVING clause now raises a SQLSyntaxError:

df.sql("""
  SELECT
    key,
    SUM(value) AS total, 
    COUNT(key) AS n_keys
  FROM self
  HAVING n_keys > 2
""")

# SQLSyntaxError: HAVING clause not valid outside of GROUP BY

In this case the caller can add the missing GROUP BY to fix the query:

df.sql("""
  SELECT 
    key,
    SUM(value) AS total, 
    COUNT(key) AS n_keys
  FROM self
  GROUP BY key
  HAVING n_keys > 2
""")

# shape: (1, 3)
# ┌─────┬───────┬────────┐
# │ key ┆ total ┆ n_keys │
# │ --- ┆ ---   ┆ ---    │
# │ str ┆ i64   ┆ u32    │
# ╞═════╪═══════╪════════╡
# │ bbb ┆ -15   ┆ 3      │
# └─────┴───────┴────────┘

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.21%. Comparing base (46edcd8) to head (228c7cf).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #19320      +/-   ##
==========================================
- Coverage   80.23%   80.21%   -0.02%     
==========================================
  Files        1523     1523              
  Lines      209529   209529              
  Branches     2434     2434              
==========================================
- Hits       168115   168083      -32     
- Misses      40859    40891      +32     
  Partials      555      555              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alippai]

Most engines don't allow select count(col1) as n from df where n > 2, actually all I know require to write select count(col1) as n from df having n > 2.

It's still LGTM, raising an error is much better than ignoring a filter

[alexander-beedie]

Most engines don't allow select count(col1) as n from df where n > 2, actually all I know require to write select count(col1) as n from df having n > 2.

Ahh, I didn't mean you could literally swap HAVING for WHERE (because you can't usually have aggregates in WHERE clauses), but rather that you can easily write the given query with a follow-on WHERE clause instead, eg:

select * from (select count(key) as n from df) where n > 2

I'll slightly edit/clarify the statement to make that clearer, and take a look at how much work it is to add the bare HAVING support later.

[alippai]

This only works if it’s part of the projection. You might want to filter on SUM and return COUNT. Ofc you can have more columns in the inner projection but it’s really getting verbose (however this is what I exactly do now)

[alippai]

Thanks for the fix!