also looking at this
fix(client): refuse a write whose predicate the builder cannot read
#1109
2 files
+251
-5
| @@ -7053,9 +7053,32 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7053 | 7053 | return this |
| 7054 | 7054 | } |
| 7055 | 7055 | |
| 7056 | // A lone SQL fragment: `where(raw('id = 1'))`, `where(sql\`...\`)`. | |
| 7057 | // The declared signature accepts SqlFragment, so this is an | |
| 7058 | // advertised call — and it matched no branch, fell through to the | |
| 7059 | // bare `return this` below, and left the UPDATE with no WHERE at all. | |
| 7060 | // Every row was rewritten, silently, and the full count was reported | |
| 7061 | // as success. Handled before the object branch because a bound | |
| 7062 | // expression is an object and would otherwise be read as the column | |
| 7063 | // map `{ sql: ... }`. See #1101. | |
| 7064 | if (isRawExpression(expr)) { | |
| 7065 | sqlText = `${sqlText} ${getWhereKeyword()} ${expr.raw}` | |
| 7066 | built = _sql.unsafe(sqlText, params) | |
| 7067 | return this | |
| 7068 | } | |
| 7069 | if (isBoundSqlExpression(expr)) { | |
| 7070 | const rendered = renderBoundSqlExpression(expr, params.length + 1) | |
| 7071 | sqlText = `${sqlText} ${getWhereKeyword()} ${rendered.text}` | |
| 7072 | params.push(...rendered.parameters) | |
| 7073 | built = _sql.unsafe(sqlText, params) | |
| 7074 | return this | |
| 7075 | } | |
| 7076 | ||
| 7056 | 7077 | // Handle object format: where({ column: value }) |
| 7057 | if (expr && typeof expr === 'object' && !('raw' in expr)) { | |
| 7078 | if (expr && typeof expr === 'object') { | |
| 7058 | 7079 | const keys = Object.keys(expr) |
| 7080 | if (keys.length === 0) | |
| 7081 | throw new TypeError('[query-builder] updateTable.where({}): an empty object is not a filter. Pass a condition, or drop the where() call if updating every row is intended.') | |
| 7059 | 7082 | const len = keys.length |
| 7060 | 7083 | const baseIdx = params.length |
| 7061 | 7084 | const conditions: string[] = Array.from({ length: len }) |
| @@ -7065,8 +7088,19 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7065 | 7088 | } |
| 7066 | 7089 | sqlText = `${sqlText} ${getWhereKeyword()} ${conditions.join(' AND ')}` |
| 7067 | 7090 | built = _sql.unsafe(sqlText, params) |
| 7091 | return this | |
| 7068 | 7092 | } |
| 7069 | return this | |
| 7093 | ||
| 7094 | // Nothing above could turn this into a predicate. Refuse it. | |
| 7095 | // | |
| 7096 | // Returning `this` unchanged meant `where(undefined)` and | |
| 7097 | // `where(null)` produced an UPDATE with no WHERE, so a filter the | |
| 7098 | // caller believed they had applied rewrote the whole table without | |
| 7099 | // an error. A write builder is the wrong place to read "I could not | |
| 7100 | // understand your filter" as "match every row". The select builder | |
| 7101 | // dropped this same fall-through in #1083; the write builders kept | |
| 7102 | // it. See #1101. | |
| 7103 | 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.`) | |
| 7070 | 7104 | }, |
| 7071 | 7105 | whereNull(column: string) { |
| 7072 | 7106 | const keyword = SQL_PATTERNS.WHERE.test(sqlText) ? 'AND' : 'WHERE' |
| @@ -7246,9 +7280,31 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7246 | 7280 | built = null |
| 7247 | 7281 | return this |
| 7248 | 7282 | } |
| 7283 | // A lone SQL fragment, handled before the object branch for the same | |
| 7284 | // reason as in updateTable.where: a bound expression is an object and | |
| 7285 | // would otherwise be read as the column map `{ sql: ... }`. Rendering | |
| 7286 | // it here also fixes it — routed through applyWhere() below, a | |
| 7287 | // fragment was interpolated as a bound value, so Postgres rejected | |
| 7288 | // `where(raw('id = 1'))` with "invalid input syntax for type | |
| 7289 | // boolean". The signature accepts SqlFragment, so it has to work. | |
| 7290 | if (isRawExpression(expr)) { | |
| 7291 | sqlText += ` ${getWhereKeyword()} ${expr.raw}` | |
| 7292 | built = null | |
| 7293 | return this | |
| 7294 | } | |
| 7295 | if (isBoundSqlExpression(expr)) { | |
| 7296 | const rendered = renderBoundSqlExpression(expr, delParams.length + 1) | |
| 7297 | sqlText += ` ${getWhereKeyword()} ${rendered.text}` | |
| 7298 | delParams.push(...rendered.parameters) | |
| 7299 | built = null | |
| 7300 | return this | |
| 7301 | } | |
| 7302 | ||
| 7249 | 7303 | // Object format: where({ id: 1 }) |
| 7250 | if (expr && typeof expr === 'object' && !('raw' in expr)) { | |
| 7304 | if (expr && typeof expr === 'object') { | |
| 7251 | 7305 | const keys = Object.keys(expr) |
| 7306 | if (keys.length === 0) | |
| 7307 | throw new TypeError('[query-builder] deleteFrom.where({}): an empty object is not a filter. Pass a condition, or drop the where() call if deleting every row is intended.') | |
| 7252 | 7308 | const conditions: string[] = [] |
| 7253 | 7309 | for (const key of keys) { |
| 7254 | 7310 | const paramIndex = delParams.length + 1 |
| @@ -7259,8 +7315,16 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7259 | 7315 | built = null |
| 7260 | 7316 | return this |
| 7261 | 7317 | } |
| 7262 | built = applyWhere(({} as any), ensureDelBuilt(), expr) | |
| 7263 | return this | |
| 7318 | ||
| 7319 | // Nothing above could turn this into a predicate. | |
| 7320 | // | |
| 7321 | // This previously fell through to applyWhere({}, ..., expr), whose | |
| 7322 | // first line is `if (!expr) return q` — so `where(undefined)` and | |
| 7323 | // `where(null)` appended nothing and the DELETE removed every row, | |
| 7324 | // returning the count as though the filter had been honoured. See | |
| 7325 | // #1101, and #1083 where the select builder lost the same | |
| 7326 | // fall-through. | |
| 7327 | 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.`) | |
| 7264 | 7328 | }, |
| 7265 | 7329 | whereNull(column: string) { |
| 7266 | 7330 | sqlText += ` ${getWhereKeyword()} ${quoteId(String(column))} IS NULL` |