Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add an $onUpdate method to columns #1509

Merged
merged 9 commits into from
Mar 27, 2024

Conversation

Angelelz
Copy link
Collaborator

This will close #956. Also related to #843.

Upon merging this PR, the user will now be able to register a function that returns the value to be inserted automatically to the column on UPDATE queries.
This will work very similar to $default()/$defaultFn() but it would work for updates.
Please keep in mind that by extension, if you register a function in this method, this function will also be used for inserts only if no .default() or .$default() were registered.

Usage

export const users = pgTable("users", {
  id: serial("id").unique().primaryKey(),
  name: text("name").notNull(),
  managerId: integer("user_id").references((): AnyPgColumn => users.id),
  createdAt: timestamp("created_at", { precision: 3 }).default(
    sql`current_timestamp(3)`,
  ),
  updatedAt: timestamp("updatedAt").$onUpdateFn(() => new Date()),
});

the updatedAt column will automatically be updated each time the row is updated.

@Angelelz Angelelz marked this pull request as ready for review November 19, 2023 03:29
@cah4a
Copy link

cah4a commented Nov 20, 2023

Any progress here?

@faysalhaque
Copy link

need this feature...

@raikusy
Copy link

raikusy commented Nov 22, 2023

Any progress with this?

@cah4a
Copy link

cah4a commented Nov 23, 2023

Looks like this is working for MySQL:

export const users = mysqlTable("users", {
  ...
  updatedAt: datetime("updatedAt").default(sql`CURRENT_TIMESTAMP ON UPDATE SET CURRENT_TIMESTAMP`),
});

drizzle-orm/src/mysql-core/dialect.ts Outdated Show resolved Hide resolved
integration-tests/tests/pg.test.ts Outdated Show resolved Hide resolved
integration-tests/tests/pg.test.ts Outdated Show resolved Hide resolved
@Angelelz Angelelz requested a review from dankochetov November 24, 2023 02:55
@raikusy
Copy link

raikusy commented Dec 4, 2023

Please release this 🙏

@buzinas
Copy link

buzinas commented Dec 20, 2023

Is there anything we can do to unblock this feature?

@jakeleventhal
Copy link

  ...

Is there a postgres workaround

@Angelelz
Copy link
Collaborator Author

Angelelz commented Jan 6, 2024

  ...

Is there a postgres workaround

Yes, the Postgres way of dealing with this is with stored procedures

@raikusy
Copy link

raikusy commented Jan 8, 2024

[Postgres] Anyone looking for an workaround until this is merged, run this on your DB -

CREATE OR REPLACE FUNCTION trigger_set_timestamp ()
	RETURNS TRIGGER
	AS $$
BEGIN
	NEW.updated_at = now();
	RETURN NEW;
END;
$$
LANGUAGE plpgsql;

@joshsmith
Copy link

Is there an update on this?

@John-Dennehy
Copy link

Looks like this is working for MySQL:

export const users = mysqlTable("users", {
  ...
  updatedAt: datetime("updatedAt").default(sql`CURRENT_TIMESTAMP ON UPDATE SET CURRENT_TIMESTAMP`),
});

For anyone else that found this didn't work, I got it working on Planetscale/MySQL by removing "SET" from the string:

updatedAt: datetime("updated_at").default(sql`CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP`)

@noah-haub
Copy link

Any progress on this? Would be a super helpful feature!

@marcuslary
Copy link

what is blocking the release of this feature?

@summaarum
Copy link

Actually been waiting on this since December. @dankochetov please have another look at this :)

@naquiroz
Copy link

@dankochetov @AndriiSherman could review this PR please?

@naquiroz
Copy link

Does anyone have a temporal approach that works in postgres, while this PR is not merged?

@predragnikolic
Copy link

@naquiroz see #956 (comment)

@naquiroz
Copy link

naquiroz commented Mar 18, 2024

@naquiroz see #956 (comment)

@predragnikolic I saw that but I had zero clue on how to implement that in drizzle. I want to keep it versioned in a migration but I had no idea how to do it

@TommyHPettersen
Copy link

Is there any status on this? Have been waiting for this feature

@arielweinberger
Copy link

Folks, Drizzle is making a lot of noise and it has great potential, but not receiving a single comment or acknowledgement from a teammate on a PR open for 5 months does not look good.

@hauserkristof
Copy link

@AndriiSherman , @dankochetov can you. please take a look at this?
@tamasbelinszky @tamasbelinkszky , me and the Wattly team are waiting for this for so long!

Thanks!

@predragnikolic
Copy link

predragnikolic commented Mar 21, 2024

There is a subscribe button, please use it.

I am sure that a lot of people are subscribed and waiting for an official update from drizzle devs.

It doesn't help flooding this thread(and people email inbox) with messages like above.

Thank you!

@AndriiSherman AndriiSherman self-assigned this Mar 27, 2024
@AndriiSherman AndriiSherman changed the base branch from main to beta March 27, 2024 09:57
@AndriiSherman AndriiSherman changed the base branch from beta to main March 27, 2024 09:57
@AndriiSherman
Copy link
Member

Will try to release today

@AndriiSherman AndriiSherman merged commit 77763ae into drizzle-team:main Mar 27, 2024
7 checks passed
}

/**
* Alias for {@link $defaultFn}.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here^

Suggested change
* Alias for {@link $defaultFn}.
* Alias for {@link $onUpdateFn}.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I'll fix before release

@houmark
Copy link

houmark commented Mar 28, 2024

This was released today.

I feel like there's a lot of limitations to this as is, which greatly affects the potential use cases possible.

a) It would be nice to have the current updating record/item as a possible context to pass to the update function, so you can use values from other fields to update the field. I guess you could do a select query inside, but that seems unnecessary if the current updating item is right there.

b) It does not seem possible to use an async function to return the new value? Use case: Creating a hash value based on the value of other fields for example using sha256 using web cryptos crypto.subtle.digest which is promised based.

@AlexBlokh
Copy link
Contributor

@houmark yeah, that makes sense

@houmark
Copy link

houmark commented Apr 6, 2024

Also after having thought about it a bit more, I think this method is either badly named or is being utilized for more than it should. From the description of this PR:

Please keep in mind that by extension, if you register a function in this method, this function will also be used for inserts only if no .default() or .$default() were registered.

At this point this method is no longer "on update" but also "on insert" in special cases. But .default() already covers that, so why is this method also doing the same? What if I only want it "on update", there seems to be now way at this time (unless I can also add an "empty" .default() in the same chain, but does that not make this API a bit too complex and error prone?).

And okay, if it is for both, then it should be named accordingly because the current name is very explicit "onUpdate", but it's not "just" on update.

Either that, or I misunderstood the description/docs (I did not have a chance to test this out yet - mainly because this method seem to be really worth using for +90% use cases I have).

@AlexBlokh
Copy link
Contributor

@houmark seems like we should have $default, $onUpdate and $onUpsert

@houmark
Copy link

houmark commented Apr 6, 2024

That could be one option yes, and I like that idea as it covers every use case with no overlap, but $onUpdate should not be doing any insert related stuff under any circumstances imho. If people want to do that, then either use both functions or the upsert one.

For me it's still way more limiting that the things mentioned in my first comments are not possible. This forces me to do JS logic stuff on every update prior to updating and that can become annoying if you have many of those in different places but you want 1-2 fields to always end up with a specific value - for example a hash based on other field values or something similar.

@tposch
Copy link

tposch commented Aug 20, 2024

$onUpdate(() => new Date()) is using the application server's time - not the database time (i.e. current_timestamp). I really need this value to be the database time as we have multiple application servers and we don't necessarily validate their times.

I tried $onUpdate(() => sql`now()`) and a few other variants but always got the same error:

TypeError: value.toISOString is not a function

If anyone has figured out how to use $onUpdate() with current_timestamp, I'd love to hear how.

EDIT: If you set the timestamp mode: 'string' then it works:

    updatedAt: timestamp('updated_at', { withTimezone: true, mode: 'string' })
      .notNull()
      .defaultNow()
      .$onUpdate(() => sql`current_timestamp`),

@tanishqmanuja
Copy link

tanishqmanuja commented Aug 28, 2024

IDK why but its not working for me,

image

drizzle-orm: 0.33.0
drizzle-kit: 0.24.2

Can someone suggest a workaround ?

@AlexBlokh
Copy link
Contributor

@tanishqmanuja .$onUpdate is a runtime function, it's not related to schema generation, it gets invoked in runtime by query builder

it works and works as expected, I don't see a reason for you to use it here

@tanishqmanuja
Copy link

Ah got it, thanks @AlexBlokh. But its weird that drizzle-kit studio doesn't trigger these $onUpdate functions.

drizzle-studio-onupdate.mp4

@AlexBlokh
Copy link
Contributor

I'm not sure we've implemented that yet, please created that as a separate issue, we'll gladly fix!

@tanishqmanuja
Copy link

I don't see a reason for you to use it here,

What should be used here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: Prisma's updatedAt behavior. onUpdate API suggestion