also looking at this
fix(client): an empty list is a predicate, not the absence of one
#1087
2 files
+131
-5
| @@ -184,6 +184,38 @@ function renderSelectColumn(col: unknown): string { | ||
| 184 | 184 | ) |
| 185 | 185 | } |
| 186 | 186 | |
| 187 | /** | |
| 188 | * Constant predicates, for the cases where a collection is empty. | |
| 189 | * | |
| 190 | * `1 = 0` and `1 = 1` take no parameters and parse identically on SQLite, | |
| 191 | * Postgres and MySQL, which is the point: the alternatives are engine-specific. | |
| 192 | * `IN ()` is the example that motivated these — SQLite parses it and matches | |
| 193 | * nothing, while Postgres and MySQL reject it as a syntax error, so the same | |
| 194 | * call site behaved differently depending on where it ran. | |
| 195 | */ | |
| 196 | const FALSE_PREDICATE = '1 = 0' | |
| 197 | const TRUE_PREDICATE = '1 = 1' | |
| 198 | ||
| 199 | /** | |
| 200 | * Render `col IN (…)` / `col NOT IN (…)`, including the empty case. | |
| 201 | * | |
| 202 | * An empty `IN` is FALSE (nothing is a member of the empty set) and an empty | |
| 203 | * `NOT IN` is TRUE (everything is a non-member). Those constants are not | |
| 204 | * interchangeable and the mirror is easy to get backwards: writing FALSE for | |
| 205 | * `NOT IN` would silently drop every row from any query whose exclusion list | |
| 206 | * happened to come back empty — the same shape of silent, widening-or-narrowing | |
| 207 | * failure as the `IN ()` bug itself. | |
| 208 | * | |
| 209 | * Shared rather than inlined at each call site because there are four of them | |
| 210 | * (`whereIn`, `orWhereIn`, `whereNotIn`, `orWhereNotIn`) and they have already | |
| 211 | * drifted once. | |
| 212 | */ | |
| 213 | function renderInPredicate(column: string, values: any[], negated: boolean, placeholders: string): string { | |
| 214 | if (values.length === 0) | |
| 215 | return negated ? TRUE_PREDICATE : FALSE_PREDICATE | |
| 216 | return `${column} ${negated ? 'NOT IN' : 'IN'} (${placeholders})` | |
| 217 | } | |
| 218 | ||
| 187 | 219 | // Pre-compiled regex patterns for performance |
| 188 | 220 | const SQL_PATTERNS = { |
| 189 | 221 | SELECT_STAR: /^SELECT\s+\*/i, |
| @@ -4793,7 +4825,20 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 4793 | 4825 | return this as any |
| 4794 | 4826 | }, |
| 4795 | 4827 | whereAny(cols: string[], op: WhereOperator, value: any) { |
| 4796 | if (cols.length === 0) return this as any | |
| 4828 | // An empty disjunction is FALSE, not absent. Returning `this` emitted | |
| 4829 | // the identity of the wrong connective, so a `.filter()` that removed | |
| 4830 | // every column WIDENED the query to all the rows it existed to | |
| 4831 | // exclude — silently, and in the dangerous direction. | |
| 4832 | // | |
| 4833 | // `whereAll([])` and `whereNone([])` keep their no-op just below, | |
| 4834 | // because TRUE genuinely is their identity. The asymmetry is the point, | |
| 4835 | // not an inconsistency to tidy up. | |
| 4836 | if (cols.length === 0) { | |
| 4837 | const kw = SQL_PATTERNS.WHERE.test(text) ? 'AND' : 'WHERE' | |
| 4838 | text += ` ${kw} (${FALSE_PREDICATE})` | |
| 4839 | built = null | |
| 4840 | return this as any | |
| 4841 | } | |
| 4797 | 4842 | const keyword = SQL_PATTERNS.WHERE.test(text) ? 'AND' : 'WHERE' |
| 4798 | 4843 | const idx = whereParams.length + 1 |
| 4799 | 4844 | const conds = cols.map((c, i) => `${c} ${op} ${getPlaceholder(idx + i)}`) |
| @@ -4876,7 +4921,7 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 4876 | 4921 | const keyword = SQL_PATTERNS.WHERE.test(text) ? 'AND' : 'WHERE' |
| 4877 | 4922 | if (Array.isArray(values)) { |
| 4878 | 4923 | const placeholders = getPlaceholders(values.length, whereParams.length + 1) |
| 4879 | text += ` ${keyword} ${column} IN (${placeholders})` | |
| 4924 | text += ` ${keyword} ${renderInPredicate(column, values, false, placeholders)}` | |
| 4880 | 4925 | whereParams.push(...values) |
| 4881 | 4926 | } |
| 4882 | 4927 | else { |
| @@ -4889,7 +4934,7 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 4889 | 4934 | validateIdentifier(String(column), 'orWhereIn(column)') |
| 4890 | 4935 | if (Array.isArray(values)) { |
| 4891 | 4936 | const placeholders = getPlaceholders(values.length, whereParams.length + 1) |
| 4892 | text += ` OR ${column} IN (${placeholders})` | |
| 4937 | text += ` OR ${renderInPredicate(column, values, false, placeholders)}` | |
| 4893 | 4938 | whereParams.push(...values) |
| 4894 | 4939 | } |
| 4895 | 4940 | else { |
| @@ -4903,7 +4948,7 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 4903 | 4948 | const keyword = SQL_PATTERNS.WHERE.test(text) ? 'AND' : 'WHERE' |
| 4904 | 4949 | if (Array.isArray(values)) { |
| 4905 | 4950 | const placeholders = getPlaceholders(values.length, whereParams.length + 1) |
| 4906 | text += ` ${keyword} ${column} NOT IN (${placeholders})` | |
| 4951 | text += ` ${keyword} ${renderInPredicate(column, values, true, placeholders)}` | |
| 4907 | 4952 | whereParams.push(...values) |
| 4908 | 4953 | } |
| 4909 | 4954 | else { |
| @@ -4916,7 +4961,7 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 4916 | 4961 | validateIdentifier(String(column), 'orWhereNotIn(column)') |
| 4917 | 4962 | if (Array.isArray(values)) { |
| 4918 | 4963 | const placeholders = getPlaceholders(values.length, whereParams.length + 1) |
| 4919 | text += ` OR ${column} NOT IN (${placeholders})` | |
| 4964 | text += ` OR ${renderInPredicate(column, values, true, placeholders)}` | |
| 4920 | 4965 | whereParams.push(...values) |
| 4921 | 4966 | } |
| 4922 | 4967 | else { |
| @@ -0,0 +1,81 @@ | ||
| 1 | import { describe, expect, it } from 'bun:test' | |
| 2 | import { buildDatabaseSchema, createQueryBuilder } from '../src' | |
| 3 | ||
| 4 | /** | |
| 5 | * Regression coverage for the two smaller defects reported in | |
| 6 | * stacksjs/bun-query-builder#1083. | |
| 7 | * | |
| 8 | * `whereIn(col, [])` emitted `IN ()`. SQLite parses that and matches nothing, | |
| 9 | * which is the sane reading; Postgres and MySQL reject it as a syntax error. So | |
| 10 | * one call site behaved differently depending on which engine it ran against — | |
| 11 | * portable-looking code that was not portable. | |
| 12 | * | |
| 13 | * `whereAny([], op, value)` returned the builder untouched, so the predicate | |
| 14 | * vanished entirely. An empty column list is nearly always a `.filter()` that | |
| 15 | * removed everything, and the consequence was MORE rows than intended: the | |
| 16 | * query silently widened to include everything the filter existed to exclude. | |
| 17 | * | |
| 18 | * Both now emit a constant predicate. `1 = 0` and `1 = 1` take no parameters | |
| 19 | * and parse identically on all three engines. | |
| 20 | * | |
| 21 | * The mirror case is the one to be careful about: an empty `NOT IN` is TRUE, | |
| 22 | * not FALSE. Nothing is a member of the empty set, so everything is a | |
| 23 | * non-member. Writing `1 = 0` there would silently drop every row from any | |
| 24 | * query whose exclusion list happened to come back empty — the same class of | |
| 25 | * silent, direction-flipping bug being fixed here. | |
| 26 | */ | |
| 27 | ||
| 28 | const schema = buildDatabaseSchema({ | |
| 29 | Row: { | |
| 30 | name: 'Row', | |
| 31 | table: 'm', | |
| 32 | primaryKey: 'id', | |
| 33 | attributes: { id: { validation: { rule: {} } }, s: { validation: { rule: {} } } }, | |
| 34 | }, | |
| 35 | } as any) | |
| 36 | ||
| 37 | const sqlOf = (build: (q: any) => any): string => | |
| 38 | String(build(createQueryBuilder<typeof schema>({ schema }).selectFrom('m').selectAll()).toSQL()) | |
| 39 | ||
| 40 | describe('empty collections emit a constant predicate (#1083)', () => { | |
| 41 | it('whereIn with an empty list matches nothing instead of emitting IN ()', () => { | |
| 42 | const sql = sqlOf(q => q.whereIn('id', [])) | |
| 43 | ||
| 44 | expect(sql).not.toContain('IN ()') | |
| 45 | expect(sql).toContain('1 = 0') | |
| 46 | }) | |
| 47 | ||
| 48 | it('whereNotIn with an empty list matches everything', () => { | |
| 49 | // THE TRAP. Excluding nothing excludes nothing — this must be TRUE. | |
| 50 | // `1 = 0` here would silently return zero rows whenever an exclusion list | |
| 51 | // came back empty. | |
| 52 | const sql = sqlOf(q => q.whereNotIn('id', [])) | |
| 53 | ||
| 54 | expect(sql).toContain('1 = 1') | |
| 55 | expect(sql).not.toContain('1 = 0') | |
| 56 | }) | |
| 57 | ||
| 58 | it('whereAny with no columns matches nothing instead of vanishing', () => { | |
| 59 | const sql = sqlOf(q => q.where('s', '=', 'p').whereAny([], 'like', '%x%')) | |
| 60 | ||
| 61 | expect(sql).toContain('1 = 0') | |
| 62 | // The earlier filter must survive alongside it. | |
| 63 | expect(sql).toContain('s = ') | |
| 64 | }) | |
| 65 | ||
| 66 | it('whereAll and whereNone with no columns stay no-ops', () => { | |
| 67 | // TRUE genuinely is their identity, so dropping the term is correct here. | |
| 68 | // The asymmetry with whereAny is deliberate. | |
| 69 | const baseline = sqlOf(q => q.where('s', '=', 'p')) | |
| 70 | ||
| 71 | expect(sqlOf(q => q.where('s', '=', 'p').whereAll([], 'like', '%x%'))).toBe(baseline) | |
| 72 | expect(sqlOf(q => q.where('s', '=', 'p').whereNone([], 'like', '%x%'))).toBe(baseline) | |
| 73 | }) | |
| 74 | ||
| 75 | it('leaves a non-empty IN untouched', () => { | |
| 76 | const sql = sqlOf(q => q.whereIn('id', [1, 2, 3])) | |
| 77 | ||
| 78 | expect(sql).toContain('IN (') | |
| 79 | expect(sql).not.toContain('1 = 0') | |
| 80 | }) | |
| 81 | }) | |