ReviewOS

stacks/bun-query-builder

whereRaw(sql, [value]) type-checks and is accepted, then throws once another clause binds and on every write, and the docs teach that form

#1146
Closed glennmichael123 opened this 9 days ago ·
9 days ago

ModelQueryBuilder.whereRaw(sql, ...params) takes its bindings as separate arguments. Passing them as an array, whereRaw(sql, [value]), type-checks, is accepted silently, and gives three different outcomes depending on the rest of the query:

  • Reads with no other binding: works, by accident.
  • Reads once any other clause binds a value: throws.
  • Every write (delete(), update(), increment()): throws, even when it is the only binding.

The docs teach that array form, so it is the form people reach for.

Reproduction

Standalone: bun:sqlite in memory, configureOrm and createModel, nothing else. bun add bun-query-builder@0.2.69, then bun repro.ts.

// Standalone repro: bun-query-builder@0.2.69 ModelQueryBuilder.whereRaw is
// variadic (`whereRaw(sql, ...params)`), so the Knex-style array form
// `whereRaw(sql, [value])` stores rawParams = [[value]] and the compiled
// params become a nested array.
//
// Run: bun install && bun repro.ts
import { Database } from 'bun:sqlite'
import { configureOrm, createModel } from 'bun-query-builder'
import pkg from 'bun-query-builder/package.json' with { type: 'json' }

const db = new Database(':memory:')
db.run(`CREATE TABLE users (
  id INTEGER PRIMARY KEY AUTOINCREMENT,
  name TEXT NOT NULL,
  country TEXT NOT NULL
)`)
db.run(`INSERT INTO users (name, country) VALUES ('Ada', 'US'), ('ada', 'UK'), ('Grace', 'US')`)

configureOrm({ database: db })

const User = createModel({
  name: 'User',
  table: 'users',
  primaryKey: 'id',
  attributes: {
    name: { type: 'string', fillable: true },
    country: { type: 'string', fillable: true },
  },
} as const)

console.log(`bun ${Bun.version}, bun-query-builder ${pkg.version}\n`)

async function run(label: string, build: () => any, exec: (q: any) => Promise<unknown>) {
  console.log(`--- ${label}`)
  const q = build()
  const { sql, params } = q.toSql()
  console.log('toSql().sql    :', sql)
  console.log('toSql().params :', JSON.stringify(params))
  try {
    const result = await exec(q)
    const shown = Array.isArray(result) ? result.map((r: any) => r.toJSON?.() ?? r) : result
    console.log('result         : OK', JSON.stringify(shown))
  }
  catch (err) {
    console.log(`result         : THROWS ${(err as Error).constructor.name}: ${(err as Error).message}`)
  }
  console.log()
}

const SQL = 'LOWER(name) = ?'

// (a) Knex-style array form, only binding -> works by accident
await run(
  "(a) whereRaw(sql, ['ada'])  [array form, only binding]",
  () => User.query().whereRaw(SQL, ['ada']),
  q => q.get(),
)

// (b) Knex-style array form followed by where() -> throws
await run(
  "(b) whereRaw(sql, ['ada']).where('country', 'US')  [array form + where]",
  () => User.query().whereRaw(SQL, ['ada']).where('country', 'US'),
  q => q.get(),
)

// (b2) same, but the array param is not the first binding
await run(
  "(b2) where('country', 'US').whereRaw(sql, ['ada'])  [array not first]",
  () => User.query().where('country', 'US').whereRaw(SQL, ['ada']),
  q => q.get(),
)

// (c) documented variadic form + where() -> works
await run(
  "(c) whereRaw(sql, 'ada').where('country', 'US')  [variadic form + where]",
  () => User.query().whereRaw(SQL, 'ada').where('country', 'US'),
  q => q.get(),
)

// (d) array form, only binding, but on a write path: delete()/update() go
// through SqliteExecutor.run(sql, params) (not spread), so even a single
// array param fails there.
await run(
  "(d) whereRaw(sql, ['grace']).delete()  [array form, only binding, write path; toSql() shows the SELECT, delete() reuses its WHERE params]",
  () => User.query().whereRaw(SQL, ['grace']),
  q => q.delete(),
)

// (e) variadic form on the same write path -> works
await run(
  "(e) whereRaw(sql, 'grace').delete()  [variadic form, write path; toSql() shows the SELECT, delete() reuses its WHERE params]",
  () => User.query().whereRaw(SQL, 'grace'),
  q => q.delete(),
)

Output, verbatim:

bun 1.3.14, bun-query-builder 0.2.69

--- (a) whereRaw(sql, ['ada'])  [array form, only binding]
toSql().sql    : SELECT * FROM users WHERE LOWER(name) = ?
toSql().params : [["ada"]]
result         : OK [{"id":1,"name":"Ada","country":"US"},{"id":2,"name":"ada","country":"UK"}]

--- (b) whereRaw(sql, ['ada']).where('country', 'US')  [array form + where]
toSql().sql    : SELECT * FROM users WHERE LOWER(name) = ? AND country = ?
toSql().params : [["ada"],"US"]
result         : THROWS Error: SQLite query expected 2 values, received 1

--- (b2) where('country', 'US').whereRaw(sql, ['ada'])  [array not first]
toSql().sql    : SELECT * FROM users WHERE country = ? AND LOWER(name) = ?
toSql().params : ["US",["ada"]]
result         : THROWS TypeError: Binding expected string, TypedArray, boolean, number, bigint or null

--- (c) whereRaw(sql, 'ada').where('country', 'US')  [variadic form + where]
toSql().sql    : SELECT * FROM users WHERE LOWER(name) = ? AND country = ?
toSql().params : ["ada","US"]
result         : OK [{"id":1,"name":"Ada","country":"US"}]

--- (d) whereRaw(sql, ['grace']).delete()  [array form, only binding, write path; toSql() shows the SELECT, delete() reuses its WHERE params]
toSql().sql    : SELECT * FROM users WHERE LOWER(name) = ?
toSql().params : [["grace"]]
result         : THROWS TypeError: Binding expected string, TypedArray, boolean, number, bigint or null

--- (e) whereRaw(sql, 'grace').delete()  [variadic form, write path; toSql() shows the SELECT, delete() reuses its WHERE params]
toSql().sql    : SELECT * FROM users WHERE LOWER(name) = ?
toSql().params : ["grace"]
result         : OK 1

Cause

Read at main 17ac15fe1ba29f16d51bf9cde2e99c65f3e158b7, the same code as the published 0.2.69:

  • packages/bun-query-builder/src/orm.ts:2596 and 2601: whereRaw/orWhereRaw(fragment: string, ...params: unknown[]) store params unchecked. With an array argument, rawParams is [[value]]. The _wheres entry type allows it (rawParams?: unknown[], orm.ts:2405).
  • orm.ts:2770, buildWhereClauses: params.push(...w.rawParams) spreads one level only, so the nested array becomes a single binding.
  • orm.ts:671/675: SqliteExecutor.all/get spread params into .all(...params). With exactly one binding that is .all([value]), and bun:sqlite accepts a single array as the whole binding list. That is the only reason (a) works.
  • orm.ts:679: SqliteExecutor.run passes params unspread, sqliteDb.run(sql, params). That is why every write path fails, (d), even with one binding.
  • The published types (dist/orm.d.ts:676-677) declare ...params: unknown[], so an array argument is a valid unknown and TypeScript raises nothing. This is from reading the signature. I did not run tsc against it.

The docs teach the failing form

  • docs/guide/where.md:192: .whereRaw('LOWER(email) = ?', ['john@example.com'])
  • docs/api/reference.md:70: `whereRaw(sql)` | .whereRaw('LOWER(name) = ?', ['john'])

Both examples are on db.selectFrom(...), the client builder, but that builder's whereRaw takes a single SqlFragment, client.ts:932, so the second argument is not part of its signature at all. Its runtime at client.ts:5479 ignores it. I read this in the source and did not run it. Anyone who learns whereRaw from these pages and then uses the ORM builder writes the array form.

What would help, roughly in order of preference

  1. Accept both forms in ModelQueryBuilder.whereRaw/orWhereRaw: const bindings = params.length === 1 && Array.isArray(params[0]) ? params[0] : params. The array form is what the docs show and what Knex users expect.
  2. Or reject it clearly: throw TypeError('whereRaw bindings are separate arguments: whereRaw(sql, a, b), not whereRaw(sql, [a, b])').
  3. Types: add an overload whereRaw(fragment: string, params: readonly unknown[]) if (1), or narrow the rest type so an array does not type-check if (2).
  4. Docs: fix docs/guide/where.md:192 and docs/api/reference.md:70 so the example matches the builder it is written against.

A regression test is the six cases above.

Context

In a Stacks app, trail text search ran Trail.query().whereRaw('id IN (SELECT rowid FROM trails_fts WHERE trails_fts MATCH ?)', [match]). It passed every local check, all of which were curl requests with no other filter. In production it returned 500 for browser traffic, because the endpoint also applies a country filter inferred from Accept-Language or cf-ipcountry, which adds a second binding. The app-side fix was whereRaw(sql, match). I found no existing issue for it.

Environment

bun-query-builder 0.2.69 (npm latest; main at 17ac15f is unchanged), Bun 1.3.14, SQLite.

Sign in to comment on this issue.