also looking at this
fix(client): place clauses by statement structure, not by text search
#1129
3 files
+392
-72
| @@ -11,7 +11,7 @@ import { bunSql, getOrCreateBunSql, resetConnection } from './db' | ||
| 11 | 11 | import { resolvePivot } from './pivot' |
| 12 | 12 | import { singularizerFor } from './inflect' |
| 13 | 13 | import type { WhereTerm } from './sql-fragments' |
| 14 | import { FALSE_PREDICATE, renderInPredicate, renderWhereTerms } from './sql-fragments' | |
| 14 | import { FALSE_PREDICATE, renderInPredicate, renderWhereTerms, scanTopLevelKeywords } from './sql-fragments' | |
| 15 | 15 | |
| 16 | 16 | export { resetConnection } |
| 17 | 17 | |
| @@ -2925,6 +2925,49 @@ function sleep(ms: number): Promise<void> { | ||
| 2925 | 2925 | const reorderCache = new Map<string, string>() |
| 2926 | 2926 | const REORDER_CACHE_MAX = 500 |
| 2927 | 2927 | |
| 2928 | /** | |
| 2929 | * Clause keywords, as one table per scan site, hoisted to module scope. | |
| 2930 | * | |
| 2931 | * A regex literal is constructed each time its expression is evaluated, so | |
| 2932 | * building these inside the builder cost ~19 allocations per query. They hold | |
| 2933 | * no state between calls: `scanTopLevelKeywords` sets `lastIndex` before every | |
| 2934 | * exec, which is the only thing the sticky flag carries. | |
| 2935 | */ | |
| 2936 | const REORDER_KEYWORDS = [ | |
| 2937 | { key: 'GROUP_BY', pattern: /GROUP\s+BY\b/iy }, | |
| 2938 | { key: 'ORDER_BY', pattern: /ORDER\s+BY\b/iy }, | |
| 2939 | { key: 'HAVING', pattern: /HAVING\b/iy }, | |
| 2940 | { key: 'OFFSET', pattern: /OFFSET\b/iy }, | |
| 2941 | { key: 'LIMIT', pattern: /LIMIT\b/iy }, | |
| 2942 | { key: 'WHERE', pattern: /WHERE\b/iy }, | |
| 2943 | ] | |
| 2944 | ||
| 2945 | const TAIL_CLAUSE = [ | |
| 2946 | { key: 'tail', pattern: /GROUP\s+BY\b/iy }, | |
| 2947 | { key: 'tail', pattern: /ORDER\s+BY\b/iy }, | |
| 2948 | { key: 'tail', pattern: /HAVING\b/iy }, | |
| 2949 | { key: 'tail', pattern: /LIMIT\b/iy }, | |
| 2950 | { key: 'tail', pattern: /OFFSET\b/iy }, | |
| 2951 | { key: 'tail', pattern: /WINDOW\b/iy }, | |
| 2952 | { key: 'tail', pattern: /UNION\b/iy }, | |
| 2953 | { key: 'tail', pattern: /INTERSECT\b/iy }, | |
| 2954 | { key: 'tail', pattern: /EXCEPT\b/iy }, | |
| 2955 | { key: 'tail', pattern: /FOR\s+UPDATE\b/iy }, | |
| 2956 | { key: 'tail', pattern: /FOR\s+SHARE\b/iy }, | |
| 2957 | { key: 'tail', pattern: /LOCK\s+IN\s+SHARE\s+MODE\b/iy }, | |
| 2958 | ] | |
| 2959 | ||
| 2960 | const JOIN_CUT = [ | |
| 2961 | { key: 'cut', pattern: /GROUP\s+BY\b/iy }, | |
| 2962 | { key: 'cut', pattern: /ORDER\s+BY\b/iy }, | |
| 2963 | { key: 'cut', pattern: /WHERE\b/iy }, | |
| 2964 | { key: 'cut', pattern: /HAVING\b/iy }, | |
| 2965 | { key: 'cut', pattern: /LIMIT\b/iy }, | |
| 2966 | { key: 'cut', pattern: /OFFSET\b/iy }, | |
| 2967 | { key: 'cut', pattern: /UNION\b/iy }, | |
| 2968 | ] | |
| 2969 | ||
| 2970 | ||
| 2928 | 2971 | function reorderSelectClauses(sql: string): string { |
| 2929 | 2972 | const hit = reorderCache.get(sql) |
| 2930 | 2973 | if (hit !== undefined) return hit |
| @@ -2940,56 +2983,13 @@ function computeReorderedClauses(sql: string): string { | ||
| 2940 | 2983 | // must be checked before single-word prefixes that would |
| 2941 | 2984 | // otherwise short-circuit them (e.g. "ORDER" alone isn't a clause |
| 2942 | 2985 | // start; "ORDER BY" is). |
| 2943 | const KEYWORDS: Array<{ key: 'WHERE' | 'GROUP_BY' | 'HAVING' | 'ORDER_BY' | 'LIMIT' | 'OFFSET', tokens: RegExp }> = [ | |
| 2944 | { key: 'GROUP_BY', tokens: /^GROUP\s+BY\b/i }, | |
| 2945 | { key: 'ORDER_BY', tokens: /^ORDER\s+BY\b/i }, | |
| 2946 | { key: 'HAVING', tokens: /^HAVING\b/i }, | |
| 2947 | { key: 'OFFSET', tokens: /^OFFSET\b/i }, | |
| 2948 | { key: 'LIMIT', tokens: /^LIMIT\b/i }, | |
| 2949 | { key: 'WHERE', tokens: /^WHERE\b/i }, | |
| 2950 | ] | |
| 2951 | 2986 | |
| 2952 | const positions: Array<{ key: string, start: number }> = [] | |
| 2953 | let depth = 0 | |
| 2954 | let inString = false | |
| 2955 | let stringChar = '' | |
| 2956 | ||
| 2957 | for (let i = 0; i < sql.length; i++) { | |
| 2958 | const ch = sql[i] | |
| 2959 | if (inString) { | |
| 2960 | if (ch === stringChar) { | |
| 2961 | // SQL escapes a quote by doubling it ('it''s'). Skip the pair. | |
| 2962 | if (sql[i + 1] === stringChar) { i++; continue } | |
| 2963 | inString = false | |
| 2964 | } | |
| 2965 | continue | |
| 2966 | } | |
| 2967 | if (ch === '\'' || ch === '"' || ch === '`') { | |
| 2968 | inString = true | |
| 2969 | stringChar = ch | |
| 2970 | continue | |
| 2971 | } | |
| 2972 | if (ch === '(') { depth++; continue } | |
| 2973 | if (ch === ')') { depth--; continue } | |
| 2974 | if (depth !== 0) continue | |
| 2975 | // Keyword must be word-boundary-led — i.e. preceded by whitespace | |
| 2976 | // (or start of string, which never matches a clause). | |
| 2977 | if (i === 0 || !/\s/.test(sql[i - 1])) continue | |
| 2978 | // Cheap pre-filter: every clause keyword starts with G/O/H/L/W. Skip the | |
| 2979 | // slice + regex battery (the hot part of this scan) for any other char. | |
| 2980 | const lead = sql.charCodeAt(i) & ~32 // ASCII uppercase | |
| 2981 | if (lead !== 71 && lead !== 79 && lead !== 72 && lead !== 76 && lead !== 87) continue | |
| 2982 | const rest = sql.slice(i) | |
| 2983 | for (const { key, tokens } of KEYWORDS) { | |
| 2984 | const m = rest.match(tokens) | |
| 2985 | if (!m) continue | |
| 2986 | positions.push({ key, start: i }) | |
| 2987 | // Advance past the matched keyword so we don't re-detect it on | |
| 2988 | // the next iteration of the outer loop. | |
| 2989 | i += m[0].length - 1 | |
| 2990 | break | |
| 2991 | } | |
| 2992 | } | |
| 2987 | // Shared with the WHERE and JOIN splices, so all three agree on what counts | |
| 2988 | // as statement structure rather than caller text. This one already skipped | |
| 2989 | // string literals; it did not skip comments, so `orderByRaw(raw('id desc /* | |
| 2990 | // where clause */'))` had the comment's `where` promoted to a clause and the | |
| 2991 | // statement cut at it. See #1121. | |
| 2992 | const positions = scanTopLevelKeywords(sql, REORDER_KEYWORDS) | |
| 2993 | 2993 | |
| 2994 | 2994 | // Nothing to reorder if zero or one clause: zero clauses means the |
| 2995 | 2995 | // SQL is just SELECT…FROM, and a single clause is already in |
| @@ -3493,19 +3493,15 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 3493 | 3493 | } |
| 3494 | 3494 | |
| 3495 | 3495 | // Index of the first TOP-LEVEL clause that must follow WHERE, or -1. |
| 3496 | // Paren-depth scan so a subquery's own ORDER BY/LIMIT doesn't match. | |
| 3497 | const TAIL_CLAUSE = /\(|\)|\b(?:GROUP\s+BY|HAVING|ORDER\s+BY|LIMIT|OFFSET|WINDOW|UNION|INTERSECT|EXCEPT|FOR\s+UPDATE|FOR\s+SHARE|LOCK\s+IN\s+SHARE\s+MODE)\b/gi | |
| 3496 | // | |
| 3497 | // Shared scanner: paren depth alone was not enough, because a raw fragment | |
| 3498 | // is caller text. `selectRaw(raw("'a limit 3 b' as t"))` put `limit` inside | |
| 3499 | // a string literal, the WHERE was spliced into the middle of it, and the | |
| 3500 | // statement stayed valid with no WHERE at all — returning every row. See | |
| 3501 | // #1121. | |
| 3498 | 3502 | const firstTailIndex = (s: string): number => { |
| 3499 | TAIL_CLAUSE.lastIndex = 0 | |
| 3500 | let depth = 0 | |
| 3501 | let m: RegExpExecArray | null | |
| 3502 | // eslint-disable-next-line no-cond-assign | |
| 3503 | while ((m = TAIL_CLAUSE.exec(s))) { | |
| 3504 | if (m[0] === '(') { depth++ } | |
| 3505 | else if (m[0] === ')') { depth = Math.max(0, depth - 1) } | |
| 3506 | else if (depth === 0) { return m.index } | |
| 3507 | } | |
| 3508 | return -1 | |
| 3503 | const hits = scanTopLevelKeywords(s, TAIL_CLAUSE, 1) | |
| 3504 | return hits.length > 0 ? hits[0].start : -1 | |
| 3509 | 3505 | } |
| 3510 | 3506 | |
| 3511 | 3507 | /** |
| @@ -3690,17 +3686,15 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 3690 | 3686 | // end of `text`, so `.where(...).join(...)` emitted `... WHERE ... JOIN ...` |
| 3691 | 3687 | // (invalid on every dialect). Also invalidates `built`. Paren-depth scan so |
| 3692 | 3688 | // a subquery's inner WHERE doesn't get matched. See #1030. |
| 3689 | // Where the JOIN goes: before the first top-level trailing clause. | |
| 3690 | // | |
| 3691 | // Same shared scanner, and the same reason. This one also spelled its | |
| 3692 | // multi-word keywords with a single literal space, so `GROUP\n BY` in a | |
| 3693 | // fragment was missed entirely — the patterns below take any whitespace. | |
| 3694 | // See #1121. | |
| 3693 | 3695 | const insertJoin = (joinClause: string) => { |
| 3694 | const re = /\(|\)|\b(?:WHERE|GROUP BY|HAVING|ORDER BY|LIMIT|OFFSET|UNION)\b/gi | |
| 3695 | let depth = 0 | |
| 3696 | let cut = -1 | |
| 3697 | let mm: RegExpExecArray | null | |
| 3698 | // eslint-disable-next-line no-cond-assign | |
| 3699 | while ((mm = re.exec(text))) { | |
| 3700 | if (mm[0] === '(') { depth++ } | |
| 3701 | else if (mm[0] === ')') { depth = Math.max(0, depth - 1) } | |
| 3702 | else if (depth === 0) { cut = mm.index; break } | |
| 3703 | } | |
| 3696 | const hits = scanTopLevelKeywords(text, JOIN_CUT, 1) | |
| 3697 | const cut = hits.length > 0 ? hits[0].start : -1 | |
| 3704 | 3698 | text = cut >= 0 |
| 3705 | 3699 | ? `${text.slice(0, cut)}${joinClause} ${text.slice(cut)}` |
| 3706 | 3700 | : `${text} ${joinClause}` |