also looking at this
fix(client): whereJsonContains binds the document, not its JSON text (#1091)
#1100
3 files
+172
-3
| @@ -4944,12 +4944,27 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 4944 | 4944 | const dialect = config.dialect |
| 4945 | 4945 | const idx = whereParams.length + 1 |
| 4946 | 4946 | if (dialect === 'postgres') { |
| 4947 | // Bind the document itself, NOT `JSON.stringify(document)`. Bun's | |
| 4948 | // driver already JSON-encodes a value bound to a jsonb parameter, so | |
| 4949 | // stringifying first encoded it twice and Postgres received the jsonb | |
| 4950 | // *string* `"[\"bun\"]"` where the array `["bun"]` was meant. A jsonb | |
| 4951 | // string never `@>`-contains anything, so the predicate was vacuously | |
| 4952 | // false and every call returned zero rows, silently. See #1091. | |
| 4953 | // | |
| 4954 | // That encoding covers objects, arrays and strings. Numbers and | |
| 4955 | // booleans are bound as int4/bool instead, which `@>` has no operator | |
| 4956 | // for (`operator does not exist: jsonb @> integer`), so those are | |
| 4957 | // lifted to a jsonb scalar with to_jsonb(). | |
| 4958 | const needsJsonbLift = typeof json === 'number' || typeof json === 'boolean' | |
| 4959 | const operand = needsJsonbLift | |
| 4960 | ? `to_jsonb(${getPlaceholder(idx)})` | |
| 4961 | : getPlaceholder(idx) | |
| 4947 | 4962 | // operator (`@>`, default) or function (`jsonb_contains`) per config. |
| 4948 | 4963 | if (config.sql?.jsonContainsMode === 'function') |
| 4949 | pushWhere('AND', `jsonb_contains(${column}, ${getPlaceholder(idx)})`) | |
| 4964 | pushWhere('AND', `jsonb_contains(${column}, ${operand})`) | |
| 4950 | 4965 | else |
| 4951 | pushWhere('AND', `${column} @> ${getPlaceholder(idx)}`) | |
| 4952 | whereParams.push(JSON.stringify(json)) | |
| 4966 | pushWhere('AND', `${column} @> ${operand}`) | |
| 4967 | whereParams.push(json) | |
| 4953 | 4968 | } |
| 4954 | 4969 | else if (isMysqlLike(dialect)) { |
| 4955 | 4970 | pushWhere('AND', `JSON_CONTAINS(${column}, ${getPlaceholder(idx)})`) |
| @@ -0,0 +1,121 @@ | ||
| 1 | /** | |
| 2 | * whereJsonContains against a live Postgres — stacksjs/bun-query-builder#1091. | |
| 3 | * | |
| 4 | * The bug: `whereJsonContains` pushed `JSON.stringify(json)`, but Bun's driver | |
| 5 | * already JSON-encodes a value bound to a jsonb parameter. The document was | |
| 6 | * therefore encoded twice and Postgres received the jsonb *string* | |
| 7 | * `"[\"bun\"]"` where the array `["bun"]` was meant. A jsonb string never | |
| 8 | * `@>`-contains anything, so the predicate was vacuously false and every call | |
| 9 | * returned zero rows, for every input, silently. | |
| 10 | * | |
| 11 | * This has to execute. The emitted SQL was correct throughout (`tags @> $1`) — | |
| 12 | * only the bound parameter was wrong, and `toSQL()` does not expose parameters, | |
| 13 | * so no SQL-text assertion can observe the difference. The existing #1026 suite | |
| 14 | * asserted the text and stayed green across the entire lifetime of this bug. | |
| 15 | * | |
| 16 | * Note also that every negative case ("no row should match") passed while the | |
| 17 | * bug was live, because the answer was always zero rows. Only positive cases — | |
| 18 | * containment that genuinely holds — can fail here, so every case below asserts | |
| 19 | * a row comes back except where it explicitly checks a non-match. | |
| 20 | * | |
| 21 | * Runs in a subprocess: `config.dialect` is global and other test files pin it | |
| 22 | * to sqlite, which would mask the network-driver path entirely. | |
| 23 | */ | |
| 24 | ||
| 25 | import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' | |
| 26 | import { tmpdir } from 'node:os' | |
| 27 | import { join, resolve } from 'node:path' | |
| 28 | import process from 'node:process' | |
| 29 | import { describe, expect, it } from 'bun:test' | |
| 30 | import { PG_URL, probePostgres } from './pg' | |
| 31 | ||
| 32 | const pgAvailable = await probePostgres() | |
| 33 | ||
| 34 | describe.skipIf(!pgAvailable)('whereJsonContains against live Postgres (#1091)', () => { | |
| 35 | it('matches rows for documents and scalars, in both containment modes', () => { | |
| 36 | const srcEntry = resolve(import.meta.dir, '../src/index.ts') | |
| 37 | const dir = mkdtempSync(join(tmpdir(), 'qb-jc1091-')) | |
| 38 | const scriptPath = join(dir, 'probe.ts') | |
| 39 | ||
| 40 | writeFileSync(scriptPath, ` | |
| 41 | import { SQL } from 'bun' | |
| 42 | import { setConfig, resetConnection, createQueryBuilder, buildDatabaseSchema, buildSchemaMeta } from ${JSON.stringify(srcEntry)} | |
| 43 | ||
| 44 | const URL = ${JSON.stringify(PG_URL)} | |
| 45 | const failures = [] | |
| 46 | const check = (label, got, want) => { | |
| 47 | const g = JSON.stringify(got), w = JSON.stringify(want) | |
| 48 | if (g !== w) failures.push(label + ': got ' + g + ', want ' + w) | |
| 49 | } | |
| 50 | ||
| 51 | const raw = new SQL(URL) | |
| 52 | await raw.unsafe('DROP TABLE IF EXISTS _qb_jsoncontains') | |
| 53 | await raw.unsafe('CREATE TABLE _qb_jsoncontains (id int primary key, tags jsonb, meta jsonb)') | |
| 54 | await raw.unsafe(\`INSERT INTO _qb_jsoncontains VALUES | |
| 55 | (1, '["bun","sql",1,true]', '{"published":true,"tier":"pro"}'), | |
| 56 | (2, '["node"]', '{"published":false}')\`) | |
| 57 | await raw.end() | |
| 58 | ||
| 59 | setConfig({ dialect: 'postgres', database: { url: URL } }) | |
| 60 | resetConnection() | |
| 61 | ||
| 62 | const models = { _qb_jsoncontains: { columns: { | |
| 63 | id: { type: 'integer', isPrimaryKey: true }, tags: { type: 'json' }, meta: { type: 'json' }, | |
| 64 | } } } | |
| 65 | const db = createQueryBuilder({ | |
| 66 | schema: buildDatabaseSchema(models), | |
| 67 | meta: buildSchemaMeta(models), | |
| 68 | autoMigration: { enabled: false }, | |
| 69 | }) | |
| 70 | ||
| 71 | const ids = async (column, operand) => { | |
| 72 | const rows = await db.selectFrom('_qb_jsoncontains').select('id') | |
| 73 | .whereJsonContains(column, operand).orderBy('id', 'asc').get() | |
| 74 | return rows.map(r => Number(r.id)) | |
| 75 | } | |
| 76 | ||
| 77 | // Documents. Each of these returned [] while #1091 was live. | |
| 78 | check('array', await ids('tags', ['bun']), [1]) | |
| 79 | check('array multi-element', await ids('tags', ['bun', 'sql']), [1]) | |
| 80 | check('object', await ids('meta', { published: true }), [1]) | |
| 81 | check('object multi-key', await ids('meta', { published: true, tier: 'pro' }), [1]) | |
| 82 | check('string', await ids('tags', 'bun'), [1]) | |
| 83 | ||
| 84 | // Scalars. Bound as int4/bool, so these need the to_jsonb lift or Postgres | |
| 85 | // rejects the statement outright rather than returning nothing. | |
| 86 | check('number', await ids('tags', 1), [1]) | |
| 87 | check('boolean', await ids('tags', true), [1]) | |
| 88 | ||
| 89 | // Non-matches still do not match — the fix must not widen the predicate. | |
| 90 | check('array non-match', await ids('tags', ['nope']), []) | |
| 91 | check('object non-match', await ids('meta', { published: 'nope' }), []) | |
| 92 | check('string non-match', await ids('tags', 'zzz'), []) | |
| 93 | check('number non-match', await ids('tags', 99), []) | |
| 94 | ||
| 95 | // jsonb_contains() is the same containment through the function form. | |
| 96 | setConfig({ sql: { jsonContainsMode: 'function' } }) | |
| 97 | check('function mode array', await ids('tags', ['bun']), [1]) | |
| 98 | check('function mode number', await ids('tags', 1), [1]) | |
| 99 | check('function mode non-match', await ids('tags', ['nope']), []) | |
| 100 | ||
| 101 | const cleanup = new SQL(URL) | |
| 102 | await cleanup.unsafe('DROP TABLE IF EXISTS _qb_jsoncontains') | |
| 103 | await cleanup.end() | |
| 104 | ||
| 105 | if (failures.length) { | |
| 106 | console.error('FAILURES:\\n' + failures.join('\\n')) | |
| 107 | process.exit(1) | |
| 108 | } | |
| 109 | console.log('OK') | |
| 110 | `) | |
| 111 | ||
| 112 | const proc = Bun.spawnSync({ cmd: ['bun', scriptPath], stdout: 'pipe', stderr: 'pipe', cwd: process.cwd(), env: { ...process.env } }) | |
| 113 | const dec = new TextDecoder() | |
| 114 | const out = dec.decode(proc.stdout).trim() | |
| 115 | const err = dec.decode(proc.stderr).trim() | |
| 116 | rmSync(dir, { recursive: true, force: true }) | |
| 117 | ||
| 118 | expect(proc.exitCode, `whereJsonContains pg probe failed.\nstdout: ${out}\nstderr: ${err}`).toBe(0) | |
| 119 | expect(out).toContain('OK') | |
| 120 | }) | |
| 121 | }) | |
| @@ -67,4 +67,37 @@ describe('whereJsonContains dialect handling (#1026)', () => { | ||
| 67 | 67 | // (`whereJsonContains('tags', ['bun'])` against a seeded table returns the |
| 68 | 68 | // matching row); an in-suite execution test is omitted because the shared |
| 69 | 69 | // lazy-connection state across test files makes it flaky. |
| 70 | ||
| 71 | /** | |
| 72 | * #1091. Postgres binds a number as int4 and a boolean as bool, neither of | |
| 73 | * which `@>` accepts (`operator does not exist: jsonb @> integer`), so a | |
| 74 | * scalar operand is lifted to a jsonb scalar. Documents are bound directly | |
| 75 | * and must NOT be lifted — the driver already encodes those as jsonb. | |
| 76 | * | |
| 77 | * These assert the SQL shape only. The bug this guards against lived | |
| 78 | * entirely in the bound parameter, which `toSQL()` does not expose, so the | |
| 79 | * real guard is the execution test in client.json-contains.pg.test.ts. | |
| 80 | */ | |
| 81 | it('Postgres: lifts a number or boolean operand with to_jsonb (#1091)', () => { | |
| 82 | config.dialect = 'postgres' as any | |
| 83 | config.sql = { ...config.sql, jsonContainsMode: 'operator' } | |
| 84 | const num = String((qb() as any).selectFrom('posts').whereJsonContains('tags', 1).toSQL()) | |
| 85 | expect(num).toContain('tags @> to_jsonb($1)') | |
| 86 | const bool = String((qb() as any).selectFrom('posts').whereJsonContains('tags', true).toSQL()) | |
| 87 | expect(bool).toContain('tags @> to_jsonb($1)') | |
| 88 | ||
| 89 | config.sql = { ...config.sql, jsonContainsMode: 'function' } | |
| 90 | const fn = String((qb() as any).selectFrom('posts').whereJsonContains('tags', 1).toSQL()) | |
| 91 | expect(fn).toContain('jsonb_contains(tags, to_jsonb($1))') | |
| 92 | }) | |
| 93 | ||
| 94 | it('Postgres: binds a document operand directly, without to_jsonb (#1091)', () => { | |
| 95 | config.dialect = 'postgres' as any | |
| 96 | config.sql = { ...config.sql, jsonContainsMode: 'operator' } | |
| 97 | for (const operand of [['bun'], { published: true }, 'bun']) { | |
| 98 | const s = String((qb() as any).selectFrom('posts').whereJsonContains('tags', operand).toSQL()) | |
| 99 | expect(s).toContain('tags @> $1') | |
| 100 | expect(s).not.toContain('to_jsonb') | |
| 101 | } | |
| 102 | }) | |
| 70 | 103 | }) |