|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#21227] `create` and `bulkCreate` answer the rows they STORED, on every |
| 5 | + * dialect: one record per written row, in the written order. |
| 6 | + * |
| 7 | + * # The defect |
| 8 | + * |
| 9 | + * Both doors ran `builder.insert(...).returning('*')` and answered what the |
| 10 | + * statement answered. MySQL has no `RETURNING`: knex's MySQL compiler drops the |
| 11 | + * clause with a `.returning() is not supported by mysql` warning and answers |
| 12 | + * `[insertId]`, which is `0` for this driver's string primary key and ONE |
| 13 | + * element whatever the row count. Measured on live MySQL 8.0.46 at `62b90d74`, |
| 14 | + * every row stored correctly each time: |
| 15 | + * |
| 16 | + * | call | answered | |
| 17 | + * |:--|:--| |
| 18 | + * | `create` (generated id) | `0` | |
| 19 | + * | `create` (supplied id) | `0` | |
| 20 | + * | `bulkCreate`, 3 rows | `[0]` (length 1) | |
| 21 | + * | `bulkCreate`, 1 row | `[0]` | |
| 22 | + * |
| 23 | + * One layer up, the auth adapter answers what `create` answers, so sign-up on |
| 24 | + * MySQL answered `400 FAILED_TO_CREATE_USER` with the user row stored and no |
| 25 | + * account row; and the engine's one-result-per-row guard refuses a multi-row |
| 26 | + * `bulkCreate` AFTER every row has landed. |
| 27 | + * |
| 28 | + * # What is asserted, per cell |
| 29 | + * |
| 30 | + * The answer is compared with the row read back through the driver's own |
| 31 | + * `findOne`, and it must carry `done: false`, a value only the column's |
| 32 | + * DEFAULT supplies (the payload never names `done`): an answer that echoed the |
| 33 | + * payload fails here, and so does `0`. |
| 34 | + * |
| 35 | + * Cells: SQLite always; live PostgreSQL and MySQL where provisioned (the |
| 36 | + * `Temporal Conformance (live PG + MySQL)` job runs this package against both) |
| 37 | + * and declared un-run otherwise. SQLite and PostgreSQL answer from `RETURNING`, |
| 38 | + * unchanged by the fix, and are the control: they passed before it and must |
| 39 | + * pass after it, in ONE statement each. MySQL reads back, in two. |
| 40 | + * |
| 41 | + * One more cell, always run: SQLite with the read-back path forced |
| 42 | + * (`insertReturnsStoredRows` overridden to `false`), so the read-back's own |
| 43 | + * logic (written order, the written tenant scope, the caller's transaction, |
| 44 | + * the refusal when a written row is gone) is measured on every CI run, not |
| 45 | + * only on the job that has a MySQL server. |
| 46 | + */ |
| 47 | + |
| 48 | +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; |
| 49 | +import { SqlDriver, type SqlDriverConfig } from '../src/index.js'; |
| 50 | +import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js'; |
| 51 | + |
| 52 | +const TABLE = 'os21227_stored_row'; |
| 53 | + |
| 54 | +const OBJECT = { |
| 55 | + name: TABLE, |
| 56 | + fields: { |
| 57 | + organization_id: { type: 'string' }, |
| 58 | + title: { type: 'string' }, |
| 59 | + qty: { type: 'number' }, |
| 60 | + // Filled by the column DEFAULT, never by a payload in this file: the field |
| 61 | + // that tells a stored row from an echo of what was sent. |
| 62 | + done: { type: 'boolean', defaultValue: false }, |
| 63 | + meta: { type: 'json' }, |
| 64 | + }, |
| 65 | +} as any; |
| 66 | + |
| 67 | +/** SQLite, with the read-back path the MySQL family takes. */ |
| 68 | +class ReadBackPathSqlDriver extends SqlDriver { |
| 69 | + protected override get insertReturnsStoredRows(): boolean { |
| 70 | + return false; |
| 71 | + } |
| 72 | +} |
| 73 | + |
| 74 | +interface Face { |
| 75 | + label: string; |
| 76 | + make(): SqlDriver; |
| 77 | + /** Whether this face answers by reading back (two statements) rather than from RETURNING. */ |
| 78 | + readsBack: boolean; |
| 79 | + /** Whether a trigger can delete the row its own INSERT wrote (SQLite only). */ |
| 80 | + sqliteTriggers: boolean; |
| 81 | +} |
| 82 | + |
| 83 | +function suite(face: Face) { |
| 84 | + describe(`sql-driver — create / bulkCreate answer the stored row (${face.label}) [#21227]`, () => { |
| 85 | + let driver: SqlDriver; |
| 86 | + const knex = () => driver.getKnex(); |
| 87 | + |
| 88 | + /** Statements this test issued against the fixture table, in order. */ |
| 89 | + let statements: string[] = []; |
| 90 | + const record = (q: { sql?: string }) => { |
| 91 | + const sql = String(q?.sql ?? ''); |
| 92 | + if (sql.includes(TABLE)) statements.push(sql.trim().split(/\s+/)[0].toLowerCase()); |
| 93 | + }; |
| 94 | + |
| 95 | + const stored = async (id: unknown) => { |
| 96 | + const row = await driver.findOne(TABLE, { where: { id } }); |
| 97 | + expect(row, `row ${String(id)} was not stored`).not.toBeNull(); |
| 98 | + return row!; |
| 99 | + }; |
| 100 | + |
| 101 | + beforeEach(async () => { |
| 102 | + driver = face.make(); |
| 103 | + await knex().schema.dropTableIfExists(TABLE); |
| 104 | + await driver.initObjects([OBJECT]); |
| 105 | + statements = []; |
| 106 | + knex().on('query', record); |
| 107 | + }); |
| 108 | + |
| 109 | + afterEach(async () => { |
| 110 | + knex().removeListener('query', record); |
| 111 | + await knex().schema.dropTableIfExists(TABLE); |
| 112 | + await driver.disconnect(); |
| 113 | + }); |
| 114 | + |
| 115 | + it('measures the path it claims to (the cell is not vacuous)', () => { |
| 116 | + expect((driver as any).insertReturnsStoredRows).toBe(!face.readsBack); |
| 117 | + }); |
| 118 | + |
| 119 | + it('create with a generated id answers the stored row', async () => { |
| 120 | + const answer = await driver.create(TABLE, { title: 'generated', qty: 1, meta: { k: [1, 2] } }); |
| 121 | + |
| 122 | + expect(typeof answer.id).toBe('string'); |
| 123 | + expect(answer.id).not.toBe(''); |
| 124 | + expect(answer.done).toBe(false); |
| 125 | + expect(answer.meta).toEqual({ k: [1, 2] }); |
| 126 | + expect(answer).toEqual(await stored(answer.id)); |
| 127 | + }); |
| 128 | + |
| 129 | + it('create with a supplied id answers that row, in one statement or two by path', async () => { |
| 130 | + const answer = await driver.create(TABLE, { id: 'given-1', title: 'supplied', qty: 2 }); |
| 131 | + |
| 132 | + expect(answer).toEqual(await stored('given-1')); |
| 133 | + expect(answer.title).toBe('supplied'); |
| 134 | + expect(answer.done).toBe(false); |
| 135 | + // The findOne above is the test's own read; the door's statements precede it. |
| 136 | + expect(statements.slice(0, face.readsBack ? 2 : 1)).toEqual(face.readsBack ? ['insert', 'select'] : ['insert']); |
| 137 | + expect(statements[face.readsBack ? 2 : 1]).toBe('select'); // the test's findOne |
| 138 | + }); |
| 139 | + |
| 140 | + it('bulkCreate answers one stored row per written row, in the written order', async () => { |
| 141 | + const answer = await driver.bulkCreate(TABLE, [ |
| 142 | + { title: 'b1' }, |
| 143 | + { id: 'given-b2', title: 'b2', qty: 2 }, |
| 144 | + { title: 'b3', qty: 3 }, |
| 145 | + ]); |
| 146 | + |
| 147 | + expect(answer).toHaveLength(3); |
| 148 | + expect(answer.map((r) => r.title)).toEqual(['b1', 'b2', 'b3']); |
| 149 | + expect(answer[1].id).toBe('given-b2'); |
| 150 | + // The batch is ONE insert, and on the read-back path ONE read. |
| 151 | + expect(statements).toEqual(face.readsBack ? ['insert', 'select'] : ['insert']); |
| 152 | + for (const row of answer) { |
| 153 | + expect(row.done).toBe(false); |
| 154 | + expect(row).toEqual(await stored(row.id)); |
| 155 | + } |
| 156 | + }); |
| 157 | + |
| 158 | + it('bulkCreate of one row answers one stored row', async () => { |
| 159 | + const answer = await driver.bulkCreate(TABLE, [{ id: 'only', title: 'single' }]); |
| 160 | + |
| 161 | + expect(answer).toHaveLength(1); |
| 162 | + expect(answer[0]).toEqual(await stored('only')); |
| 163 | + }); |
| 164 | + |
| 165 | + it('a tenanted create answers the row stamped with the caller tenant', async () => { |
| 166 | + const answer = await driver.create(TABLE, { id: 't-own', title: 'own' }, { tenantId: 'org_a' }); |
| 167 | + |
| 168 | + expect(answer.organization_id).toBe('org_a'); |
| 169 | + expect(answer).toEqual(await stored('t-own')); |
| 170 | + }); |
| 171 | + |
| 172 | + it('an admin write naming another tenant answers the row it stored, not a refusal', async () => { |
| 173 | + // `injectTenantOnInsert` never overwrites an explicit tenant, so the row |
| 174 | + // lands under org_b while the call carries org_a. A read-back scoped to |
| 175 | + // the caller's ACTIVE org would miss it. |
| 176 | + const answer = await driver.create( |
| 177 | + TABLE, |
| 178 | + { id: 't-named', title: 'named', organization_id: 'org_b' }, |
| 179 | + { tenantId: 'org_a' }, |
| 180 | + ); |
| 181 | + expect(answer.organization_id).toBe('org_b'); |
| 182 | + expect(answer).toEqual(await stored('t-named')); |
| 183 | + |
| 184 | + const batch = await driver.bulkCreate( |
| 185 | + TABLE, |
| 186 | + [ |
| 187 | + { id: 't-b1', title: 'stamped' }, |
| 188 | + { id: 't-b2', title: 'named', organization_id: 'org_c' }, |
| 189 | + ], |
| 190 | + { tenantId: 'org_a' }, |
| 191 | + ); |
| 192 | + expect(batch.map((r) => [r.id, r.organization_id])).toEqual([ |
| 193 | + ['t-b1', 'org_a'], |
| 194 | + ['t-b2', 'org_c'], |
| 195 | + ]); |
| 196 | + }); |
| 197 | + |
| 198 | + it('inside a caller transaction, create and bulkCreate answer the uncommitted rows and the rollback keeps', async () => { |
| 199 | + const trx = await driver.beginTransaction(); |
| 200 | + let single!: Record<string, unknown>; |
| 201 | + let batch!: Record<string, unknown>[]; |
| 202 | + try { |
| 203 | + single = await driver.create(TABLE, { id: 'tx-1', title: 'in tx' }, { transaction: trx }); |
| 204 | + batch = await driver.bulkCreate(TABLE, [{ id: 'tx-2', title: 'in tx' }, { id: 'tx-3', title: 'in tx' }], { |
| 205 | + transaction: trx, |
| 206 | + }); |
| 207 | + } finally { |
| 208 | + await driver.rollback(trx); |
| 209 | + } |
| 210 | + expect(single.id).toBe('tx-1'); |
| 211 | + expect(single.done).toBe(false); |
| 212 | + expect(batch.map((r) => r.id)).toEqual(['tx-2', 'tx-3']); |
| 213 | + expect(await driver.find(TABLE, {})).toEqual([]); |
| 214 | + }); |
| 215 | + |
| 216 | + if (face.readsBack && face.sqliteTriggers) { |
| 217 | + it('a written row that is gone before the read-back is refused, once, and never re-issued', async () => { |
| 218 | + await knex().raw( |
| 219 | + `create trigger os21227_vanish after insert on ${TABLE} begin delete from ${TABLE} where id = new.id; end`, |
| 220 | + ); |
| 221 | + statements = []; |
| 222 | + |
| 223 | + const err = await driver.create(TABLE, { id: 'gone', title: 'vanishes' }).then( |
| 224 | + () => null, |
| 225 | + (e: unknown) => e as { code?: string; status?: number }, |
| 226 | + ); |
| 227 | + expect(err).not.toBeNull(); |
| 228 | + expect(err!.code).toBe('DATABASE_ERROR'); |
| 229 | + expect(err!.status).toBe(500); |
| 230 | + // One insert, one read, no retry: re-issuing would duplicate a row that landed. |
| 231 | + expect(statements).toEqual(['insert', 'select']); |
| 232 | + }); |
| 233 | + } |
| 234 | + }); |
| 235 | +} |
| 236 | + |
| 237 | +for (const cell of DIALECT_CELLS) { |
| 238 | + declareDialectCell(cell, 'create / bulkCreate stored row (#21227)', (c: DialectCell) => |
| 239 | + suite({ |
| 240 | + label: c.label, |
| 241 | + make: () => new SqlDriver(c.config()), |
| 242 | + readsBack: c.id === 'mysql', |
| 243 | + sqliteTriggers: c.id === 'sqlite', |
| 244 | + }), |
| 245 | + ); |
| 246 | +} |
| 247 | + |
| 248 | +suite({ |
| 249 | + label: 'sqlite, read-back path forced', |
| 250 | + make: () => new ReadBackPathSqlDriver(dialectSqliteConfig()), |
| 251 | + readsBack: true, |
| 252 | + sqliteTriggers: true, |
| 253 | +}); |
| 254 | + |
| 255 | +function dialectSqliteConfig(): SqlDriverConfig { |
| 256 | + return DIALECT_CELLS.find((c) => c.id === 'sqlite')!.config(); |
| 257 | +} |
0 commit comments