ReviewOS

also looking at this

stacks/bun-query-builder

fix(client): choose WHERE vs AND from builder state, not a text search

#1118
Merged glennmichael123 wants to merge fix/1113-where-keyword-state into main
2 files +235 -27
packages/bun-query-builder/src/client.tsmodified+55-27
Changes to packages/bun-query-builder/src/client.ts
@@ -6966,6 +6966,31 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
69666966
69676967 let sqlText = `UPDATE ${quoteId(String(table))}`
69686968
6969 /**
6970 * Whether this builder has already emitted a predicate — which is the
6971 * thing the keyword actually depends on.
6972 *
6973 * It used to be inferred by testing the whole statement for
6974 * `/\bWHERE\b/`, and plenty of text that is not this builder's predicate
6975 * satisfies that: a subquery inside `set()`, a table named `where`, a
6976 * column named `where`. The first real predicate then came out as `AND`
6977 * and fused onto the SET expression:
6978 *
6979 * SET "flag" = (SELECT ... WHERE x.id > 3) AND "id" = ?
6980 *
6981 * which leaves the UPDATE with no WHERE at all. Postgres rejects that
6982 * only when the types happen not to line up; where the SET target is
6983 * boolean, or on SQLite, it runs against every row and writes the
6984 * predicate into the value. See #1113.
6985 *
6986 * A boolean cannot be fooled by any of that.
6987 */
6988 let hasPredicate = false
6989 const appendPredicate = (predicate: string): void => {
6990 sqlText = `${sqlText} ${hasPredicate ? 'AND' : 'WHERE'} ${predicate}`
6991 hasPredicate = true
6992 }
6993
69696994 return {
69706995 set(values) {
69716996 const keys = Object.keys(values)
@@ -6992,9 +7017,6 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
69927017 return this
69937018 },
69947019 where(expr: any, op?: string, value?: any) {
6995 // Helper to get the correct keyword (WHERE for first condition, AND for subsequent)
6996 const getWhereKeyword = () => SQL_PATTERNS.WHERE.test(sqlText) ? 'AND' : 'WHERE'
6997
69987020 // Handle 3-arg format: where('column', '=', value)
69997021 if (op !== undefined && (typeof expr === 'string' || isRawExpression(expr) || isBoundSqlExpression(expr))) {
70007022 const safeOperator = assertSafeWhereOperator(op, 'updateTable.where')
@@ -7020,14 +7042,14 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
70207042 if (safeOperator.toLowerCase() === 'in' || safeOperator.toLowerCase() === 'not in') {
70217043 const values = Array.isArray(value) ? value : [value]
70227044 const placeholders = getPlaceholders(values.length, params.length + 1)
7023 sqlText = `${sqlText} ${getWhereKeyword()} ${left} ${safeOperator.toUpperCase()} (${placeholders})`
7045 appendPredicate(`${left} ${safeOperator.toUpperCase()} (${placeholders})`)
70247046 params.push(...values)
70257047 built = _sql.unsafe(sqlText, params)
70267048 return this
70277049 }
70287050
70297051 const paramIndex = params.length + 1
7030 sqlText = `${sqlText} ${getWhereKeyword()} ${left} ${safeOperator} ${getPlaceholder(paramIndex)}`
7052 appendPredicate(`${left} ${safeOperator} ${getPlaceholder(paramIndex)}`)
70317053 params.push(value)
70327054 built = _sql.unsafe(sqlText, params)
70337055 return this
@@ -7041,14 +7063,14 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
70417063 if (safeOperator.toLowerCase() === 'in' || safeOperator.toLowerCase() === 'not in') {
70427064 const values = Array.isArray(val) ? val : [val]
70437065 const placeholders = getPlaceholders(values.length, params.length + 1)
7044 sqlText = `${sqlText} ${getWhereKeyword()} ${quoteId(String(col))} ${safeOperator.toUpperCase()} (${placeholders})`
7066 appendPredicate(`${quoteId(String(col))} ${safeOperator.toUpperCase()} (${placeholders})`)
70457067 params.push(...values)
70467068 built = _sql.unsafe(sqlText, params)
70477069 return this
70487070 }
70497071
70507072 const paramIndex = params.length + 1
7051 sqlText = `${sqlText} ${getWhereKeyword()} ${quoteId(String(col))} ${safeOperator} ${getPlaceholder(paramIndex)}`
7073 appendPredicate(`${quoteId(String(col))} ${safeOperator} ${getPlaceholder(paramIndex)}`)
70527074 params.push(val)
70537075 built = _sql.unsafe(sqlText, params)
70547076 return this
@@ -7063,13 +7085,13 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
70637085 // expression is an object and would otherwise be read as the column
70647086 // map `{ sql: ... }`. See #1101.
70657087 if (isRawExpression(expr)) {
7066 sqlText = `${sqlText} ${getWhereKeyword()} ${expr.raw}`
7088 appendPredicate(expr.raw)
70677089 built = _sql.unsafe(sqlText, params)
70687090 return this
70697091 }
70707092 if (isBoundSqlExpression(expr)) {
70717093 const rendered = renderBoundSqlExpression(expr, params.length + 1)
7072 sqlText = `${sqlText} ${getWhereKeyword()} ${rendered.text}`
7094 appendPredicate(rendered.text)
70737095 params.push(...rendered.parameters)
70747096 built = _sql.unsafe(sqlText, params)
70757097 return this
@@ -7087,7 +7109,7 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
70877109 conditions[i] = `${quoteId(keys[i])} = ${getPlaceholder(baseIdx + i + 1)}`
70887110 params.push((expr as any)[keys[i]])
70897111 }
7090 sqlText = `${sqlText} ${getWhereKeyword()} ${conditions.join(' AND ')}`
7112 appendPredicate(conditions.join(' AND '))
70917113 built = _sql.unsafe(sqlText, params)
70927114 return this
70937115 }
@@ -7104,14 +7126,12 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
71047126 throw new TypeError(`[query-builder] updateTable.where(): expected a condition, got ${expr === null ? 'null' : typeof expr}. Refusing to run an UPDATE with no WHERE — drop the where() call if updating every row is intended.`)
71057127 },
71067128 whereNull(column: string) {
7107 const keyword = SQL_PATTERNS.WHERE.test(sqlText) ? 'AND' : 'WHERE'
7108 sqlText = `${sqlText} ${keyword} ${quoteId(String(column))} IS NULL`
7129 appendPredicate(`${quoteId(String(column))} IS NULL`)
71097130 built = _sql.unsafe(sqlText, params)
71107131 return this
71117132 },
71127133 whereNotNull(column: string) {
7113 const keyword = SQL_PATTERNS.WHERE.test(sqlText) ? 'AND' : 'WHERE'
7114 sqlText = `${sqlText} ${keyword} ${quoteId(String(column))} IS NOT NULL`
7134 appendPredicate(`${quoteId(String(column))} IS NOT NULL`)
71157135 built = _sql.unsafe(sqlText, params)
71167136 return this
71177137 },
@@ -7237,13 +7257,21 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
72377257 const delParams: any[] = []
72387258 let whereCondition: any = null
72397259
7240 // First .where() emits ` WHERE `; subsequent calls emit ` AND `.
7260 // First predicate emits ` WHERE `; subsequent ones emit ` AND `.
72417261 // Without this, chained `.where('a', '=', 1).where('b', '=', 2)`
72427262 // compiled to `... WHERE a = ? WHERE b = ?` and SQLite 500'd
7243 // with `near "WHERE": syntax error`. Mirrors the same helper in
7244 // updateTable() at line ~5454.
7263 // with `near "WHERE": syntax error`.
72457264 // See https://github.com/stacksjs/bun-query-builder/issues/1015
7246 const getWhereKeyword = () => SQL_PATTERNS.WHERE.test(sqlText) ? 'AND' : 'WHERE'
7265 //
7266 // Tracked as builder state rather than read back out of the statement:
7267 // `DELETE FROM "where"` matches /\bWHERE\b/ too, and inferring it from
7268 // the text turned the first predicate into `AND` — `DELETE FROM "where"
7269 // AND "id" = ?`. Mirrors updateTable(). See #1113.
7270 let hasPredicate = false
7271 const appendPredicate = (predicate: string): void => {
7272 sqlText += ` ${hasPredicate ? 'AND' : 'WHERE'} ${predicate}`
7273 hasPredicate = true
7274 }
72477275
72487276 const ensureDelBuilt = () => {
72497277 if (built === null) {
@@ -7271,14 +7299,14 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
72717299 if (safeOperator.toLowerCase() === 'in' || safeOperator.toLowerCase() === 'not in') {
72727300 const values = Array.isArray(value) ? value : [value]
72737301 const placeholders = getPlaceholders(values.length, delParams.length + 1)
7274 sqlText += ` ${getWhereKeyword()} ${quoteId(expr)} ${safeOperator.toUpperCase()} (${placeholders})`
7302 appendPredicate(`${quoteId(expr)} ${safeOperator.toUpperCase()} (${placeholders})`)
72757303 delParams.push(...values)
72767304 built = null
72777305 return this
72787306 }
72797307
72807308 const paramIndex = delParams.length + 1
7281 sqlText += ` ${getWhereKeyword()} ${quoteId(expr)} ${safeOperator} ${getPlaceholder(paramIndex)}`
7309 appendPredicate(`${quoteId(expr)} ${safeOperator} ${getPlaceholder(paramIndex)}`)
72827310 delParams.push(value)
72837311 built = null
72847312 return this
@@ -7291,14 +7319,14 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
72917319 if (safeOperator.toLowerCase() === 'in' || safeOperator.toLowerCase() === 'not in') {
72927320 const values = Array.isArray(val) ? val : [val]
72937321 const placeholders = getPlaceholders(values.length, delParams.length + 1)
7294 sqlText += ` ${getWhereKeyword()} ${quoteId(String(col))} ${safeOperator.toUpperCase()} (${placeholders})`
7322 appendPredicate(`${quoteId(String(col))} ${safeOperator.toUpperCase()} (${placeholders})`)
72957323 delParams.push(...values)
72967324 built = null
72977325 return this
72987326 }
72997327
73007328 const paramIndex = delParams.length + 1
7301 sqlText += ` ${getWhereKeyword()} ${quoteId(String(col))} ${safeOperator} ${getPlaceholder(paramIndex)}`
7329 appendPredicate(`${quoteId(String(col))} ${safeOperator} ${getPlaceholder(paramIndex)}`)
73027330 delParams.push(val)
73037331 built = null
73047332 return this
@@ -7311,13 +7339,13 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
73117339 // `where(raw('id = 1'))` with "invalid input syntax for type
73127340 // boolean". The signature accepts SqlFragment, so it has to work.
73137341 if (isRawExpression(expr)) {
7314 sqlText += ` ${getWhereKeyword()} ${expr.raw}`
7342 appendPredicate(expr.raw)
73157343 built = null
73167344 return this
73177345 }
73187346 if (isBoundSqlExpression(expr)) {
73197347 const rendered = renderBoundSqlExpression(expr, delParams.length + 1)
7320 sqlText += ` ${getWhereKeyword()} ${rendered.text}`
7348 appendPredicate(rendered.text)
73217349 delParams.push(...rendered.parameters)
73227350 built = null
73237351 return this
@@ -7334,7 +7362,7 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
73347362 conditions.push(`${quoteId(key)} = ${getPlaceholder(paramIndex)}`)
73357363 delParams.push((expr as any)[key])
73367364 }
7337 sqlText += ` ${getWhereKeyword()} ${conditions.join(' AND ')}`
7365 appendPredicate(conditions.join(' AND '))
73387366 built = null
73397367 return this
73407368 }
@@ -7350,12 +7378,12 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial
73507378 throw new TypeError(`[query-builder] deleteFrom.where(): expected a condition, got ${expr === null ? 'null' : typeof expr}. Refusing to run a DELETE with no WHERE — drop the where() call if deleting every row is intended.`)
73517379 },
73527380 whereNull(column: string) {
7353 sqlText += ` ${getWhereKeyword()} ${quoteId(String(column))} IS NULL`
7381 appendPredicate(`${quoteId(String(column))} IS NULL`)
73547382 built = null
73557383 return this
73567384 },
73577385 whereNotNull(column: string) {
7358 sqlText += ` ${getWhereKeyword()} ${quoteId(String(column))} IS NOT NULL`
7386 appendPredicate(`${quoteId(String(column))} IS NOT NULL`)
73597387 built = null
73607388 return this
73617389 },