From c6726985516d950f926abe431bec1dc60808d69b Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Wed, 10 Jun 2026 16:41:51 -0700 Subject: [PATCH] fix(orm): restore lazy `$onUpdate` evaluation in buildUpdateSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #5780. The 0.45.x refactor of `buildUpdateSet` introduced eager evaluation of the `$onUpdate` callback for every column with one — including columns whose value was explicitly supplied in the caller's `set` object, where the result is then discarded via `??`. Side-effecting callbacks (throwing, logging, reading external state) now run on every UPDATE, which is a breaking regression from the 0.44.6 contract where the callback only fired when the column was absent. Restore lazy evaluation while keeping the 0.45.x `SQL`-type check that motivated the refactor: const onUpdateFnResult = set[colName] !== undefined ? undefined : col.onUpdateFn?.(); Applied identically across pg-core, mysql-core, sqlite-core, singlestore-core, and gel-core — the bug had the same shape in each dialect's `buildUpdateSet`. A regression test (`drizzle-orm/tests/on-update.test.ts`) exercises the pg-core path with a throwing `$onUpdate` callback to assert the callback is not invoked when the column is explicitly set, plus a second test that confirms the callback IS invoked exactly once when the column is absent. Co-Authored-By: Claude Opus 4.7 (1M context) --- drizzle-orm/src/gel-core/dialect.ts | 4 +- drizzle-orm/src/mysql-core/dialect.ts | 4 +- drizzle-orm/src/pg-core/dialect.ts | 7 ++- drizzle-orm/src/singlestore-core/dialect.ts | 4 +- drizzle-orm/src/sqlite-core/dialect.ts | 4 +- drizzle-orm/tests/on-update.test.ts | 70 +++++++++++++++++++++ 6 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 drizzle-orm/tests/on-update.test.ts diff --git a/drizzle-orm/src/gel-core/dialect.ts b/drizzle-orm/src/gel-core/dialect.ts index 6c1541286b..a2be1acb56 100644 --- a/drizzle-orm/src/gel-core/dialect.ts +++ b/drizzle-orm/src/gel-core/dialect.ts @@ -149,7 +149,9 @@ export class GelDialect { return sql.join(columnNames.flatMap((colName, i) => { const col = tableColumns[colName]!; - const onUpdateFnResult = col.onUpdateFn?.(); + // Only invoke the `$onUpdate` callback when the column is absent from + // `set` — see drizzle-team/drizzle-orm#5780. + const onUpdateFnResult = set[colName] !== undefined ? undefined : col.onUpdateFn?.(); const value = set[colName] ?? (is(onUpdateFnResult, SQL) ? onUpdateFnResult : sql.param(onUpdateFnResult, col)); const res = sql`${sql.identifier(this.casing.getColumnCasing(col))} = ${value}`; diff --git a/drizzle-orm/src/mysql-core/dialect.ts b/drizzle-orm/src/mysql-core/dialect.ts index 41e1006ae2..c496e3401b 100644 --- a/drizzle-orm/src/mysql-core/dialect.ts +++ b/drizzle-orm/src/mysql-core/dialect.ts @@ -160,7 +160,9 @@ export class MySqlDialect { columnNames.flatMap((colName, i) => { const col = tableColumns[colName]!; - const onUpdateFnResult = col.onUpdateFn?.(); + // Only invoke the `$onUpdate` callback when the column is absent + // from `set` — see drizzle-team/drizzle-orm#5780. + const onUpdateFnResult = set[colName] !== undefined ? undefined : col.onUpdateFn?.(); const value = set[colName] ?? (is(onUpdateFnResult, SQL) ? onUpdateFnResult diff --git a/drizzle-orm/src/pg-core/dialect.ts b/drizzle-orm/src/pg-core/dialect.ts index bd9d6a5c95..0dc395e168 100644 --- a/drizzle-orm/src/pg-core/dialect.ts +++ b/drizzle-orm/src/pg-core/dialect.ts @@ -160,7 +160,12 @@ export class PgDialect { return sql.join(columnNames.flatMap((colName, i) => { const col = tableColumns[colName]!; - const onUpdateFnResult = col.onUpdateFn?.(); + // Only invoke the `$onUpdate` callback when the column is absent from + // `set` — eagerly calling it for every column would re-introduce the + // regression in #5780 where side-effecting callbacks (logging, + // throwing, reading external state) fire on every UPDATE even when + // the caller explicitly provided the column value. + const onUpdateFnResult = set[colName] !== undefined ? undefined : col.onUpdateFn?.(); const value = set[colName] ?? (is(onUpdateFnResult, SQL) ? onUpdateFnResult : sql.param(onUpdateFnResult, col)); const res = sql`${sql.identifier(this.casing.getColumnCasing(col))} = ${value}`; diff --git a/drizzle-orm/src/singlestore-core/dialect.ts b/drizzle-orm/src/singlestore-core/dialect.ts index 88fe959ab2..dede93a06c 100644 --- a/drizzle-orm/src/singlestore-core/dialect.ts +++ b/drizzle-orm/src/singlestore-core/dialect.ts @@ -159,7 +159,9 @@ export class SingleStoreDialect { columnNames.flatMap((colName, i) => { const col = tableColumns[colName]!; - const onUpdateFnResult = col.onUpdateFn?.(); + // Only invoke the `$onUpdate` callback when the column is absent + // from `set` — see drizzle-team/drizzle-orm#5780. + const onUpdateFnResult = set[colName] !== undefined ? undefined : col.onUpdateFn?.(); const value = set[colName] ?? (is(onUpdateFnResult, SQL) ? onUpdateFnResult diff --git a/drizzle-orm/src/sqlite-core/dialect.ts b/drizzle-orm/src/sqlite-core/dialect.ts index 1ed20858dd..e627b40a9b 100644 --- a/drizzle-orm/src/sqlite-core/dialect.ts +++ b/drizzle-orm/src/sqlite-core/dialect.ts @@ -117,7 +117,9 @@ export abstract class SQLiteDialect { columnNames.flatMap((colName, i) => { const col = tableColumns[colName]!; - const onUpdateFnResult = col.onUpdateFn?.(); + // Only invoke the `$onUpdate` callback when the column is absent + // from `set` — see drizzle-team/drizzle-orm#5780. + const onUpdateFnResult = set[colName] !== undefined ? undefined : col.onUpdateFn?.(); const value = set[colName] ?? (is(onUpdateFnResult, SQL) ? onUpdateFnResult diff --git a/drizzle-orm/tests/on-update.test.ts b/drizzle-orm/tests/on-update.test.ts new file mode 100644 index 0000000000..a9f2dfaa4e --- /dev/null +++ b/drizzle-orm/tests/on-update.test.ts @@ -0,0 +1,70 @@ +import postgres from 'postgres'; +import { describe, expect, it } from 'vitest'; +import { pgTable, text, uuid } from '~/pg-core'; +import { drizzle } from '~/postgres-js'; +import { eq } from '~/sql'; + +// Regression for drizzle-team/drizzle-orm#5780. In 0.45.x `buildUpdateSet` +// invoked every column's `$onUpdate` callback eagerly, even when the column +// value was explicitly supplied in `set`. Callbacks with side effects +// (throwing, logging, reading external state) then ran on every UPDATE. +// +// These tests assert the 0.44.6 contract: `$onUpdate` is only invoked when +// the column is absent from `set`. The pg-core dialect is exercised here; +// the identical pattern lives in mysql-core, sqlite-core, singlestore-core, +// and gel-core, and is fixed alongside pg-core in the same patch. + +const db = drizzle(postgres('')); + +describe('$onUpdate evaluation', () => { + it('does not invoke the callback when the column is explicitly provided in set', () => { + const table = pgTable('example', { + id: uuid('id').primaryKey(), + name: text('name').notNull(), + updatedById: uuid('updated_by_id') + .$onUpdate(() => { + throw new Error('should not be called when column is explicitly set'); + }) + .notNull(), + }); + + const query = db + .update(table) + .set({ name: 'foo', updatedById: '00000000-0000-0000-0000-000000000000' }) + .where(eq(table.id, '00000000-0000-0000-0000-000000000000')); + + expect(() => query.toSQL()).not.toThrow(); + expect(query.toSQL()).toEqual({ + sql: + 'update "example" set "name" = $1, "updated_by_id" = $2 where "example"."id" = $3', + params: [ + 'foo', + '00000000-0000-0000-0000-000000000000', + '00000000-0000-0000-0000-000000000000', + ], + }); + }); + + it('invokes the callback exactly once when the column is absent from set', () => { + let invocations = 0; + const table = pgTable('example', { + id: uuid('id').primaryKey(), + name: text('name').notNull(), + updatedById: uuid('updated_by_id') + .$onUpdate(() => { + invocations++; + return '11111111-1111-1111-1111-111111111111'; + }) + .notNull(), + }); + + const query = db + .update(table) + .set({ name: 'foo' }) + .where(eq(table.id, '00000000-0000-0000-0000-000000000000')); + + const compiled = query.toSQL(); + expect(invocations).toBe(1); + expect(compiled.params).toContain('11111111-1111-1111-1111-111111111111'); + }); +});