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'); + }); +});