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

createdAt @default(now()) and updatedAt @updatedAt get different times on row creation #12572

Closed
flesler opened this issue Mar 29, 2022 · 14 comments · Fixed by #15441
Closed
Assignees
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. topic: @default(now()) topic: updatedAt
Milestone

Comments

@flesler
Copy link

flesler commented Mar 29, 2022

Bug description

I have standard table like this:

model User {
  id                     String              @id @default(cuid())
  createdAt              DateTime            @default(now())
  updatedAt              DateTime            @updatedAt
}

I always assumed that after a prisma.user.create({}), the createdAt and updatedAt would be the same dates, matching to the millisecond. I'm seeing that's not the case and updatedAt many times has a few extra millisecond(s).

How to reproduce

  1. Create the table above
  2. Add many rows and log the query result
  3. You'll see what I describe eventually

Expected behavior

On creation, createdAt and updatedAt are always identical.
I suppose if @updatedAt kind of columns would also default to CURRENT_TIMESTAMP like the default(now()) do, the problem would be resolved

Prisma information

The schema above is enough

Environment & setup

  • OS: Ubuntu 20
  • Database: PostgreSQL
  • Node.js version: v14.17.0

Prisma Version

prisma                  : 3.6.0
@prisma/client          : 3.6.0
Current platform        : debian-openssl-1.1.x
Query Engine (Node-API) : libquery-engine dc520b92b1ebb2d28dc3161f9f82e875bd35d727 (at node_modules/@prisma/engines/libquery_engine-debian-openssl-1.1.x.so.node)
Migration Engine        : migration-engine-cli dc520b92b1ebb2d28dc3161f9f82e875bd35d727 (at node_modules/@prisma/engines/migration-engine-debian-openssl-1.1.x)
Introspection Engine    : introspection-core dc520b92b1ebb2d28dc3161f9f82e875bd35d727 (at node_modules/@prisma/engines/introspection-engine-debian-openssl-1.1.x)
Format Binary           : prisma-fmt dc520b92b1ebb2d28dc3161f9f82e875bd35d727 (at node_modules/@prisma/engines/prisma-fmt-debian-openssl-1.1.x)
Default Engines Hash    : dc520b92b1ebb2d28dc3161f9f82e875bd35d727
Studio                  : 0.440.0
Preview Features        : filterJson
@flesler flesler added the kind/bug A reported bug. label Mar 29, 2022
@janpio janpio added bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. team/client Issue for team Client. topic: updatedAt topic: @default(now()) bug/2-confirmed Bug has been reproduced and confirmed. and removed bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. labels Mar 30, 2022
@janpio
Copy link
Member

janpio commented Mar 30, 2022

@default(now()) and @updatedAt are currently implemented on differnet levels. The default is from the database, while the @updatedAt is written by Prisma Client (or its Query Engine). That unfortunately leads to slight differences there sometimes.

We are aware of this limitation, and will one day clean this up. We do not know exactly how yet though (move both to Prisma Client? move both to the database?) and it is unfortunately also not the highest on our priority list. (It is also a bit more complex than just these two functions, as the "defaults can be on database and/or Prisma level" problem is wider)

@flesler
Copy link
Author

flesler commented Mar 30, 2022

@janpio would it work if you make Prisma Migrate set the column's default to CURRENT_TIMESTAMP like it does for default(now()) and maybe not set it when the row is created (as opposed to updated)?

@janpio
Copy link
Member

janpio commented Mar 30, 2022

Something like that, yes. But that would be a breaking change and actually unwanted for some users who do not want to have this functionality represented in the database - so this is something more complex we will need to look deep into, and then figure out which solutions we want to support going forward and if we e.g. need a configuration switch etc.

@flesler
Copy link
Author

flesler commented May 18, 2022

@janpio I wonder if simply supporting @updatedAt @default(now()) would be an easy sell for you. It's backwards compatible and it's honestly a fix, in the sense of you simply supporting what mosts columns already support

@janpio
Copy link
Member

janpio commented May 18, 2022

It would be ugly, but the same level of ugly as the different timestamp being created on row creation. Of course it would not fix the problem for when you later update the row that the timestamp is still calculated by different code.

The problem with the suggestion is that it would a) mean introducing defaults for this type of column (which I think is blocked right now, but most importantly b) would require us to disable some code (the @updatedAt client side logic) when a default is present. And that is really a different domain, so that's nasty :/ Does that make sense?

@flesler
Copy link
Author

flesler commented May 18, 2022

@janpio not really hehe. I think it's more of a technical hole you guys are in, which is normal and happens to all. But from a consumer perspective, it doesn't make sense to me 😄
Anyway, no need to explain, I think it looks clean (consistent basically) from the outside but if it's complex to add, then I guess not 🤷‍♀️

@okomarov
Copy link

okomarov commented Jun 9, 2022

This is also tangentially relevant to determine if an upsert was a create. In general I think the upsert should return a property that says if it was a create, an update or a simple get... but in the meantime, we CANNOT use updated_at === created_at to check if it was a create (I ran into this issue through a test which is basically wrong/flaky)

@casey-chow
Copy link

It may be useful to edit #3432 (comment) which suggests checking equality as a means of determining whether a record was created or updated in an upsert.

@okomarov
Copy link

@andrewicarlson

@aqrln aqrln changed the title createdAt and updatedAt get different times on row creation createdAt @default(now()) and updatedAt @updatedAt get different times on row creation Sep 2, 2022
@do4gr
Copy link
Member

do4gr commented Sep 6, 2022

Internal: As a sidenote, this should not be super hard to fix since currently the QE writes both the values to the db, it just initializes them at different times.

@Jolg42 Jolg42 self-assigned this Sep 7, 2022
@Jolg42
Copy link
Member

Jolg42 commented Sep 7, 2022

Can reproduce in our internal dev version 4.4.0-dev.30

When using a local database (docker on my computer) the values are the same

[
  {
    id: '123',
    createdAt: 2022-09-07T15:18:36.159Z,
    updatedAt: 2022-09-07T15:18:36.159Z
  }
]

When using a remote database, it doesn't match anymore (946Z vs 947Z)

[
  {
    id: '123',
    createdAt: 2022-09-07T15:20:44.946Z,
    updatedAt: 2022-09-07T15:20:44.947Z
  }
]

@Jolg42 Jolg42 removed their assignment Sep 7, 2022
@Jolg42
Copy link
Member

Jolg42 commented Sep 7, 2022

Note
schema

generator client {
  provider = "prisma-client-js"
}

datasource db {
  provider = "postgresql"
  url      = env("DATABASE_URL")
}

model User {
  id        String   @id @default(cuid())
  createdAt DateTime @default(now())
  updatedAt DateTime @updatedAt
}

the SQL from migrate dev is

-- CreateTable
CREATE TABLE "User" (
    "id" TEXT NOT NULL,
    "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updatedAt" TIMESTAMP(3) NOT NULL,

    CONSTRAINT "User_pkey" PRIMARY KEY ("id")
);

@flesler
Copy link
Author

flesler commented Sep 7, 2022

I think the simplest solution would be that @updatedAt supports an optional @default(now()) to be specified and Prisma doesn't provide a Date on creation if there's a default, and let's the DB handle it

@tomhoule tomhoule self-assigned this Sep 14, 2022
@tomhoule
Copy link
Contributor

This will be fixed in next release. The relevant PR is prisma/prisma-engines#3200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. topic: @default(now()) topic: updatedAt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants