Skip to content

Commit

Permalink
[All] Added requested changes: fix issue ignoring null values on update
Browse files Browse the repository at this point in the history
  • Loading branch information
@Angelelz
Angelelz committed Dec 27, 2023
1 parent 5b82489 commit fa3bd5e
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 75 deletions.
2 changes: 1 addition & 1 deletion drizzle-orm/src/mysql-core/dialect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class MySqlDialect {
const tableColumns = table[Table.Symbol.Columns];

const columnNames = Object.keys(tableColumns).filter((colName) =>
!!set[colName] || tableColumns[colName]?.onUpdateFn !== undefined
set[colName] !== undefined || tableColumns[colName]?.onUpdateFn !== undefined
);

const setSize = columnNames.length;
Expand Down
2 changes: 1 addition & 1 deletion drizzle-orm/src/pg-core/dialect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class PgDialect {
const tableColumns = table[Table.Symbol.Columns];

const columnNames = Object.keys(tableColumns).filter((colName) =>
!!set[colName] || tableColumns[colName]?.onUpdateFn !== undefined
set[colName] !== undefined || tableColumns[colName]?.onUpdateFn !== undefined
);

const setSize = columnNames.length;
Expand Down
2 changes: 1 addition & 1 deletion drizzle-orm/src/sqlite-core/dialect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export abstract class SQLiteDialect {
const tableColumns = table[Table.Symbol.Columns];

const columnNames = Object.keys(tableColumns).filter((colName) =>
!!set[colName] || tableColumns[colName]?.onUpdateFn !== undefined
set[colName] !== undefined || tableColumns[colName]?.onUpdateFn !== undefined
);

const setSize = columnNames.length;
Expand Down
50 changes: 27 additions & 23 deletions integration-tests/tests/libsql.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const usersOnUpdate = sqliteTable('users_on_update', {
name: text('name').notNull(),
updateCounter: integer('update_counter').default(sql`1`).$onUpdateFn(() => sql`update_counter + 1`),
updatedAt: integer('updated_at', { mode: 'timestamp_ms' }).$onUpdate(() => new Date()),
alwaysNull: text('always_null').$type<string | null>().$onUpdate(() => null),
// uppercaseName: text('uppercase_name').$onUpdateFn(() =>
// sql`upper(s.name)`
// ), This doesn't seem to be supported in sqlite
Expand Down Expand Up @@ -2567,11 +2568,12 @@ test.serial('test $onUpdateFn and $onUpdate works as $default', async (t) => {
await db.run(
sql`
create table ${usersOnUpdate} (
id integer primary key autoincrement,
name text not null,
update_counter integer default 1 not null,
updated_at integer
)
id integer primary key autoincrement,
name text not null,
update_counter integer default 1 not null,
updated_at integer,
always_null text
)
`,
);

Expand All @@ -2588,15 +2590,15 @@ test.serial('test $onUpdateFn and $onUpdate works as $default', async (t) => {
const response = await db.select({ ...rest }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

t.deepEqual(response, [
{ name: 'John', id: 1, updateCounter: 1 },
{ name: 'Jane', id: 2, updateCounter: 1 },
{ name: 'Jack', id: 3, updateCounter: 1 },
{ name: 'Jill', id: 4, updateCounter: 1 },
{ name: 'John', id: 1, updateCounter: 1, alwaysNull: null },
{ name: 'Jane', id: 2, updateCounter: 1, alwaysNull: null },
{ name: 'Jack', id: 3, updateCounter: 1, alwaysNull: null },
{ name: 'Jill', id: 4, updateCounter: 1, alwaysNull: null },
]);
const msDelay = 100;
const msDelay = 250;

for (const eachUser of justDates) {
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay); // This test might fail if db read is too slow. Is there a better way to test Date.now()?
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay);
}
});

Expand All @@ -2608,37 +2610,39 @@ test.serial('test $onUpdateFn and $onUpdate works updating', async (t) => {
await db.run(
sql`
create table ${usersOnUpdate} (
id integer primary key autoincrement,
name text not null,
update_counter integer default 1 not null,
updated_at integer
)
id integer primary key autoincrement,
name text not null,
update_counter integer default 1,
updated_at integer,
always_null text
)
`,
);

await db.insert(usersOnUpdate).values([
{ name: 'John' },
{ name: 'John', alwaysNull: 'this will be null after updating' },
{ name: 'Jane' },
{ name: 'Jack' },
{ name: 'Jill' },
]);
const { updatedAt, ...rest } = getTableColumns(usersOnUpdate);

await db.update(usersOnUpdate).set({ name: 'Angel' }).where(eq(usersOnUpdate.id, 1));
await db.update(usersOnUpdate).set({ updateCounter: null }).where(eq(usersOnUpdate.id, 2));

const justDates = await db.select({ updatedAt }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

const response = await db.select({ ...rest }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

t.deepEqual(response, [
{ name: 'Angel', id: 1, updateCounter: 2 },
{ name: 'Jane', id: 2, updateCounter: 1 },
{ name: 'Jack', id: 3, updateCounter: 1 },
{ name: 'Jill', id: 4, updateCounter: 1 },
{ name: 'Angel', id: 1, updateCounter: 2, alwaysNull: null },
{ name: 'Jane', id: 2, updateCounter: null, alwaysNull: null },
{ name: 'Jack', id: 3, updateCounter: 1, alwaysNull: null },
{ name: 'Jill', id: 4, updateCounter: 1, alwaysNull: null },
]);
const msDelay = 100;
const msDelay = 250;

for (const eachUser of justDates) {
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay); // This test might fail if db read is too slow. Is there a better way to test Date.now()?
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay);
}
});
55 changes: 29 additions & 26 deletions integration-tests/tests/mysql.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const usersOnUpdate = mysqlTable('users_on_update', {
updateCounter: int('update_counter').default(sql`1`).$onUpdateFn(() => sql`update_counter + 1`),
updatedAt: datetime('updated_at', { mode: 'date', fsp: 3 }).$onUpdate(() => new Date()),
uppercaseName: text('uppercase_name').$onUpdateFn(() => sql`upper(name)`),
alwaysNull: text('always_null').$type<string | null>().$onUpdateFn(() => null), // need to add $type because $onUpdate add a default value
});

const datesTable = mysqlTable('datestable', {
Expand Down Expand Up @@ -2796,12 +2797,13 @@ test.serial('test $onUpdateFn and $onUpdate works as $default', async (t) => {
await db.execute(
sql`
create table ${usersOnUpdate} (
id serial not null primary key,
name text not null,
update_counter integer default 1 not null,
updated_at datetime(3),
uppercase_name text
)
id serial not null primary key,
name text not null,
update_counter integer default 1 not null,
updated_at datetime(3),
uppercase_name text,
always_null text
)
`,
);

Expand All @@ -2818,15 +2820,15 @@ test.serial('test $onUpdateFn and $onUpdate works as $default', async (t) => {
const response = await db.select({ ...rest }).from(usersOnUpdate);

t.deepEqual(response, [
{ name: 'John', id: 1, updateCounter: 1, uppercaseName: 'JOHN' },
{ name: 'Jane', id: 2, updateCounter: 1, uppercaseName: 'JANE' },
{ name: 'Jack', id: 3, updateCounter: 1, uppercaseName: 'JACK' },
{ name: 'Jill', id: 4, updateCounter: 1, uppercaseName: 'JILL' },
{ name: 'John', id: 1, updateCounter: 1, uppercaseName: 'JOHN', alwaysNull: null },
{ name: 'Jane', id: 2, updateCounter: 1, uppercaseName: 'JANE', alwaysNull: null },
{ name: 'Jack', id: 3, updateCounter: 1, uppercaseName: 'JACK', alwaysNull: null },
{ name: 'Jill', id: 4, updateCounter: 1, uppercaseName: 'JILL', alwaysNull: null },
]);
const msDelay = 100;
const msDelay = 250;

for (const eachUser of justDates) {
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay); // This test might fail if db read is too slow. Is there a better way to test Date.now()?
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay);
}
});

Expand All @@ -2838,41 +2840,42 @@ test.serial('test $onUpdateFn and $onUpdate works updating', async (t) => {
await db.execute(
sql`
create table ${usersOnUpdate} (
id serial not null primary key,
name text not null,
update_counter integer default 1 not null,
updated_at datetime(3),
uppercase_name text
)
id serial not null primary key,
name text not null,
update_counter integer default 1 not null,
updated_at datetime(3),
uppercase_name text,
always_null text
)
`,
);

await db.insert(usersOnUpdate).values([
{ name: 'John' },
{ name: 'John', alwaysNull: 'this will will be null after updating' },
{ name: 'Jane' },
{ name: 'Jack' },
{ name: 'Jill' },
]);
const { updatedAt, ...rest } = getTableColumns(usersOnUpdate);
const initial = await db.select({ updatedAt }).from(usersOnUpdate);

await db.update(usersOnUpdate).set({ name: 'Angel' }).where(eq(usersOnUpdate.id, 1));
await db.update(usersOnUpdate).set({ name: 'Angel', uppercaseName: null }).where(eq(usersOnUpdate.id, 1));

const justDates = await db.select({ updatedAt }).from(usersOnUpdate);

const response = await db.select({ ...rest }).from(usersOnUpdate);

t.deepEqual(response, [
{ name: 'Angel', id: 1, updateCounter: 2, uppercaseName: 'ANGEL' },
{ name: 'Jane', id: 2, updateCounter: 1, uppercaseName: 'JANE' },
{ name: 'Jack', id: 3, updateCounter: 1, uppercaseName: 'JACK' },
{ name: 'Jill', id: 4, updateCounter: 1, uppercaseName: 'JILL' },
{ name: 'Angel', id: 1, updateCounter: 2, uppercaseName: null, alwaysNull: null },
{ name: 'Jane', id: 2, updateCounter: 1, uppercaseName: 'JANE', alwaysNull: null },
{ name: 'Jack', id: 3, updateCounter: 1, uppercaseName: 'JACK', alwaysNull: null },
{ name: 'Jill', id: 4, updateCounter: 1, uppercaseName: 'JILL', alwaysNull: null },
]);
const msDelay = 100;
const msDelay = 250;

t.assert(initial[0]?.updatedAt?.valueOf() !== justDates[0]?.updatedAt?.valueOf());

for (const eachUser of justDates) {
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay); // This test might fail if db read is too slow. Is there a better way to test Date.now()?
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay);
}
});
50 changes: 27 additions & 23 deletions integration-tests/tests/pg.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const usersOnUpdate = pgTable('users_on_update', {
name: text('name').notNull(),
updateCounter: integer('update_counter').default(sql`1`).$onUpdateFn(() => sql`update_counter + 1`),
updatedAt: timestamp('updated_at', { mode: 'date', precision: 3 }).$onUpdate(() => new Date()),
alwaysNull: text('always_null').$type<string | null>().$onUpdate(() => null),
// uppercaseName: text('uppercase_name').$onUpdateFn(() => sql`upper(name)`), looks like this is not supported in pg
});

Expand Down Expand Up @@ -3346,11 +3347,12 @@ test.serial('test $onUpdateFn and $onUpdate works as $default', async (t) => {
await db.execute(
sql`
create table ${usersOnUpdate} (
id serial primary key,
name text not null,
update_counter integer default 1 not null,
updated_at timestamp(3)
)
id serial primary key,
name text not null,
update_counter integer default 1 not null,
updated_at timestamp(3),
always_null text
)
`,
);

Expand All @@ -3368,15 +3370,15 @@ test.serial('test $onUpdateFn and $onUpdate works as $default', async (t) => {
const response = await db.select({ ...rest }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

t.deepEqual(response, [
{ name: 'John', id: 1, updateCounter: 1 },
{ name: 'Jane', id: 2, updateCounter: 1 },
{ name: 'Jack', id: 3, updateCounter: 1 },
{ name: 'Jill', id: 4, updateCounter: 1 },
{ name: 'John', id: 1, updateCounter: 1, alwaysNull: null },
{ name: 'Jane', id: 2, updateCounter: 1, alwaysNull: null },
{ name: 'Jack', id: 3, updateCounter: 1, alwaysNull: null },
{ name: 'Jill', id: 4, updateCounter: 1, alwaysNull: null },
]);
const msDelay = 100;
const msDelay = 250;

for (const eachUser of justDates) {
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay); // This test might fail if db read is too slow. Is there a better way to test Date.now()?
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay);
}
});

Expand All @@ -3388,16 +3390,17 @@ test.serial('test $onUpdateFn and $onUpdate works updating', async (t) => {
await db.execute(
sql`
create table ${usersOnUpdate} (
id serial primary key,
name text not null,
update_counter integer default 1 not null,
updated_at timestamp(3)
)
id serial primary key,
name text not null,
update_counter integer default 1,
updated_at timestamp(3),
always_null text
)
`,
);

await db.insert(usersOnUpdate).values([
{ name: 'John' },
{ name: 'John', alwaysNull: 'this will be null after updating' },
{ name: 'Jane' },
{ name: 'Jack' },
{ name: 'Jill' },
Expand All @@ -3407,22 +3410,23 @@ test.serial('test $onUpdateFn and $onUpdate works updating', async (t) => {
const initial = await db.select({ updatedAt }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

await db.update(usersOnUpdate).set({ name: 'Angel' }).where(eq(usersOnUpdate.id, 1));
await db.update(usersOnUpdate).set({ updateCounter: null }).where(eq(usersOnUpdate.id, 2));

const justDates = await db.select({ updatedAt }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

const response = await db.select({ ...rest }).from(usersOnUpdate).orderBy(asc(usersOnUpdate.id));

t.deepEqual(response, [
{ name: 'Angel', id: 1, updateCounter: 2 },
{ name: 'Jane', id: 2, updateCounter: 1 },
{ name: 'Jack', id: 3, updateCounter: 1 },
{ name: 'Jill', id: 4, updateCounter: 1 },
{ name: 'Angel', id: 1, updateCounter: 2, alwaysNull: null },
{ name: 'Jane', id: 2, updateCounter: null, alwaysNull: null },
{ name: 'Jack', id: 3, updateCounter: 1, alwaysNull: null },
{ name: 'Jill', id: 4, updateCounter: 1, alwaysNull: null },
]);
const msDelay = 100;
const msDelay = 250;

t.assert(initial[0]?.updatedAt?.valueOf() !== justDates[0]?.updatedAt?.valueOf());

for (const eachUser of justDates) {
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay); // This test might fail if db read is too slow. Is there a better way to test Date.now()?
t.assert(eachUser.updatedAt!.valueOf() > Date.now() - msDelay);
}
});

0 comments on commit fa3bd5e

Please sign in to comment.