ReviewOS

stacks/bun-query-builder

union/unionAll drop the other side's params and collide placeholder numbers

#1029
Closed glennmichael123 opened this 2 months ago ·
2 months ago

Summary

union(other) / unionAll(other) do text += String(other.toSQL()) — text only. The right-hand query's whereParams are never merged. On Postgres both branches independently number from $1, producing duplicate $1 and a missing param; on SQLite/MySQL the ? count won't match the param array. whereExists(subquery) has the same param-loss.

Evidence (repro)

a.where({age:1}).union(b.where({age:2})).toSQL()
-> ... WHERE age = $1 UNION ... WHERE age = $1   (right value 2 lost, $1 duplicated)

Location

src/client.ts:~4474-4483 (and whereExists).

Suggested fix

Merge the sub-builder's params into the parent's whereParams and renumber the right side's placeholders by the parent offset.

Severity: high. Found via internal code audit.

9 days ago

Fixed in bf77dce. union()/unionAll() now merge the right side's bound params and renumber its $n placeholders past the left side's on Postgres (via a new internal __rawState() accessor + appendSetOp() helper), instead of appending text only and dropping params / colliding $1. Regression test added (Postgres renumber + SQLite order); full suite green. (Set-op helper is also the natural seam for INTERSECT/EXCEPT — #1049.)

Sign in to comment on this issue.