ReviewOS

stacks/bun-query-builder

having() and havingRaw() infer HAVING vs AND from the statement text, so the first HAVING fuses onto the GROUP BY list

#1122
Closed glennmichael123 opened this 23 days ago · 0 comments
23 days ago

Found on main @ 9c496fa, verified by execution.

This is #1113 with a different keyword. The two having methods pick between HAVING and AND by scanning the statement built so far:

// client.ts:5482 (having)
const kw = /\bHAVING\b/i.test(text) ? 'AND' : 'HAVING'
// client.ts:5515 (havingRaw)
const kw = /\bHAVING\b/i.test(text) ? 'AND' : 'HAVING'

text at that point holds the select list, the FROM, the joins and any raw fragment the caller passed. A HAVING in any of those is not this builder's HAVING clause.

Repro

db.selectFrom('users').selectRaw(raw("'having a party' as t")).groupBy('id').havingRaw(raw('COUNT(id) > 3'))
SELECT *, 'having a party' as t FROM users GROUP BY id AND COUNT(id) > 3
--                                                     ^^^ should be HAVING

The aggregate condition fuses onto the GROUP BY list. Same through the non-raw form:

db.selectFrom('users').selectRaw(raw("'having a party' as t")).groupBy('id').having(['COUNT(id)', '>', 3])
SELECT GROUP BY id AND COUNT(id) > $1 having FROM users

— that second one is more mangled because the GROUP BY splice (#1121) then lands inside the select list too, so the two defects compound.

Severity

Below #1113 and #1121. Both outputs are syntax errors, so this is loud — there is no silent-wrong-results path here, unlike the UPDATE case in #1113 or the swallowed WHERE in #1121. It means "aggregate filter plus a raw select fragment mentioning the word having" simply does not work.

Suggested fix

The same boolean that #1118 introduced for the write builders: track whether this builder has emitted its HAVING, rather than looking for one in text it did not write.

Found by the same sweep as #1121; reproduced before filing.

Sign in to comment on this issue.