also looking at this
fix(client): pick the select builder's keywords from state, not from text
#1126
2 files
+198
-5
| @@ -3413,6 +3413,28 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 3413 | 3413 | // UNION/INTERSECT/EXCEPT belong to the RIGHT-hand SELECT and must render at |
| 3414 | 3414 | // the end of `text`, or their params would bind in the wrong order. |
| 3415 | 3415 | let whereTail = false |
| 3416 | /** | |
| 3417 | * Whether the operand a tail predicate would attach to already carries one. | |
| 3418 | * | |
| 3419 | * Only consulted once `whereTail` is set. It used to be inferred by testing | |
| 3420 | * the WHOLE statement for `/\bWHERE\b/`, but by then `text` holds the LEFT | |
| 3421 | * select — WHERE and all — so a predicate on the left was read as one on | |
| 3422 | * the right and the right side's first predicate came out as `AND`: | |
| 3423 | * | |
| 3424 | * SELECT * FROM a WHERE y = $1 UNION SELECT * FROM b AND x = $2 | |
| 3425 | * | |
| 3426 | * which does not parse. Set from the right-hand operand alone. See #1120. | |
| 3427 | */ | |
| 3428 | let tailHasPredicate = false | |
| 3429 | /** | |
| 3430 | * Whether this builder has emitted its HAVING. | |
| 3431 | * | |
| 3432 | * Same defect as #1113, different keyword: scanning `text` for | |
| 3433 | * `/\bHAVING\b/` also matches a raw select fragment that merely contains | |
| 3434 | * the word, so the first real HAVING was emitted as `AND` and fused onto | |
| 3435 | * the GROUP BY list. See #1122. | |
| 3436 | */ | |
| 3437 | let hasHaving = false | |
| 3416 | 3438 | |
| 3417 | 3439 | // Lazy building: don't prepare statement until execution |
| 3418 | 3440 | // built is initialized lazily to avoid expensive template tag calls on every query |
| @@ -3479,7 +3501,7 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 3479 | 3501 | if (!body) |
| 3480 | 3502 | return text |
| 3481 | 3503 | if (whereTail) { |
| 3482 | const kw = SQL_PATTERNS.WHERE.test(text) ? 'AND' : 'WHERE' | |
| 3504 | const kw = tailHasPredicate ? 'AND' : 'WHERE' | |
| 3483 | 3505 | return `${text} ${kw} ${body}` |
| 3484 | 3506 | } |
| 3485 | 3507 | const cut = firstTailIndex(text) |
| @@ -3600,7 +3622,7 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 3600 | 3622 | // the other side's bound params and renumbering its `$n` placeholders past |
| 3601 | 3623 | // ours on Postgres — previously the set-op appended text only, dropping the |
| 3602 | 3624 | // right side's params and colliding `$1`. See stacksjs/bun-query-builder#1029. |
| 3603 | const appendSetOp = (op: string, other: { toSQL: () => any, __rawState?: () => { sql: string, params: unknown[] } }) => { | |
| 3625 | const appendSetOp = (op: string, other: { toSQL: () => any, __rawState?: () => { sql: string, params: unknown[] }, __hasPredicate?: () => boolean }) => { | |
| 3604 | 3626 | // Params are one flat array in push order, so the WHERE text has to |
| 3605 | 3627 | // precede the set operator's params in the emitted string. Materialize it |
| 3606 | 3628 | // into `text` now; terms added after this point belong to the RIGHT-hand |
| @@ -3618,10 +3640,17 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 3618 | 3640 | : st.sql |
| 3619 | 3641 | text += ` ${op} ${otherSql}` |
| 3620 | 3642 | whereParams.push(...st.params) |
| 3643 | // A predicate added after this point attaches to the RIGHT operand, so | |
| 3644 | // the keyword depends on that operand alone — not on the statement, | |
| 3645 | // which by now also holds the left side's WHERE. Ask the builder; only | |
| 3646 | // scan when it is a foreign one that cannot answer. See #1120. | |
| 3647 | tailHasPredicate = other.__hasPredicate?.() ?? SQL_PATTERNS.WHERE.test(otherSql) | |
| 3621 | 3648 | } |
| 3622 | 3649 | else { |
| 3623 | 3650 | // Foreign builder without __rawState — fall back to text-only (no param merge). |
| 3624 | text += ` ${op} ${String(other.toSQL())}` | |
| 3651 | const otherSql = String(other.toSQL()) | |
| 3652 | text += ` ${op} ${otherSql}` | |
| 3653 | tailHasPredicate = other.__hasPredicate?.() ?? SQL_PATTERNS.WHERE.test(otherSql) | |
| 3625 | 3654 | } |
| 3626 | 3655 | built = null |
| 3627 | 3656 | } |
| @@ -5614,12 +5643,15 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 5614 | 5643 | having(expr: any) { |
| 5615 | 5644 | // Chained having() calls join with AND, not a second HAVING keyword |
| 5616 | 5645 | // (`HAVING a HAVING b` is invalid). See stacksjs/bun-query-builder#1034. |
| 5617 | const kw = /\bHAVING\b/i.test(text) ? 'AND' : 'HAVING' | |
| 5646 | // Read from builder state, not from `text` — a raw select fragment that | |
| 5647 | // merely contains the word made the first HAVING emit as AND. See #1122. | |
| 5648 | const kw = hasHaving ? 'AND' : 'HAVING' | |
| 5618 | 5649 | // Handle array format: ['COUNT(id)', '>', 3] |
| 5619 | 5650 | if (Array.isArray(expr)) { |
| 5620 | 5651 | const paramIdx = whereParams.length + 1 |
| 5621 | 5652 | text = `${text} ${kw} ${expr[0]} ${expr[1]} ${getPlaceholder(paramIdx)}` |
| 5622 | 5653 | whereParams.push(expr[2]) |
| 5654 | hasHaving = true | |
| 5623 | 5655 | built = null |
| 5624 | 5656 | } |
| 5625 | 5657 | // Handle object format |
| @@ -5635,20 +5667,23 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 5635 | 5667 | whereParams.push(expr[key]) |
| 5636 | 5668 | } |
| 5637 | 5669 | text = `${text} ${kw} ${conditions.join(' AND ')}` |
| 5670 | hasHaving = true | |
| 5638 | 5671 | built = null |
| 5639 | 5672 | } |
| 5640 | 5673 | } |
| 5641 | 5674 | // Handle raw expressions |
| 5642 | 5675 | else if (expr && typeof (expr as any).raw !== 'undefined') { |
| 5643 | 5676 | text += ` ${kw} ${(expr as any).raw}` |
| 5677 | hasHaving = true | |
| 5644 | 5678 | built = null |
| 5645 | 5679 | } |
| 5646 | 5680 | return this as any |
| 5647 | 5681 | }, |
| 5648 | 5682 | havingRaw(fragment: any) { |
| 5649 | 5683 | const frag = renderRawFragment(fragment, 'havingRaw(fragment)') |
| 5650 | const kw = /\bHAVING\b/i.test(text) ? 'AND' : 'HAVING' | |
| 5684 | const kw = hasHaving ? 'AND' : 'HAVING' | |
| 5651 | 5685 | text += ` ${kw} ${frag}` |
| 5686 | hasHaving = true | |
| 5652 | 5687 | built = null |
| 5653 | 5688 | return this as any |
| 5654 | 5689 | }, |
| @@ -6330,6 +6365,14 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 6330 | 6365 | __rawState() { |
| 6331 | 6366 | return { sql: reorderSelectClauses(currentSql()), params: [...whereParams] } |
| 6332 | 6367 | }, |
| 6368 | // Internal: does this builder's SQL already carry a predicate that a | |
| 6369 | // further where() would have to join with AND? Used by appendSetOp on | |
| 6370 | // the other side, so it can answer that about the right-hand operand | |
| 6371 | // instead of scanning a statement that also contains the left one. | |
| 6372 | // See stacksjs/bun-query-builder#1120. | |
| 6373 | __hasPredicate() { | |
| 6374 | return whereTerms.length > 0 || tailHasPredicate | |
| 6375 | }, | |
| 6333 | 6376 | raw() { |
| 6334 | 6377 | return (ensureBuilt() as any).raw() |
| 6335 | 6378 | }, |