ReviewOS

also looking at this

stacks/bun-query-builder

fix(client): pick the select builder's keywords from state, not from text

#1126
Merged glennmichael123 wants to merge fix/1120-1122-select-keyword-state into main
2 files +198 -5
8 days ago

Closes #1120, closes #1122.

Two sites in the select builder chose a keyword by searching the statement built so far — the defect #1113 fixed in the write builders, in the builder that kept it. Grouped because it is one change made twice, and doing them together means the select builder gets the treatment once.

#1120 — a predicate after a set operator

The WHERE/AND branch is only reachable once whereTail is set, and appendSetOp materialises the left select's WHERE into text before appending the operator. So the scan answered "does the left side have a predicate?" when the question was about the right:

SELECT * FROM a WHERE y = $1 UNION SELECT * FROM b AND x = $2

That doesn't parse — which made "filter the right-hand side of a union" unavailable to anyone who had also filtered the left.

The keyword now comes from the right-hand operand alone, recorded at appendSetOp time. Our own builders answer through a new internal __hasPredicate(); the text scan survives only as the fallback for a foreign { toSQL } that can't.

#1122 — HAVING

having() and havingRaw() scanned for /\bHAVING\b/, which a raw select fragment containing the word also satisfies. The first real HAVING emitted as AND and fused onto the GROUP BY list:

SELECT *, 'having a party' as t FROM users GROUP BY id AND COUNT(id) > 3

Now a boolean, set on each branch that actually appends — so the empty-object branch of having(), which emits nothing, correctly does not claim the keyword.

What stays true

Chained predicates still join with AND on both paths — #1015 and #1034 stay fixed — and where the right operand genuinely does carry its own predicate, AND remains correct and is still what comes out. Both are covered.

Verification

6 of the 11 new tests fail without this change. The union cases execute as well as assert on emitted text, because a keyword decision that yields valid SQL can still yield the wrong rows, and only running it settles that.

Full suite 2088 pass / 0 fail, typecheck clean, pickier 0 errors.

Still open

This does not touch #1121 — the three clause placement scanners, where a raw fragment containing a keyword inside a string literal or a comment swallows the WHERE or the JOIN and the query silently returns the wrong rows. That one is a design call rather than a boolean, and it is the more serious of the two families.

🤖 Generated with Claude Code

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.