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(quaint): upgrade tiberius to 0.12.3 #5037

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

jkomyno
Copy link
Contributor

@jkomyno jkomyno commented Nov 5, 2024

This PR contributes to ORM-314.

The current version of tiberius in prisma-engines is 0.11.8.
The most recent version available is 0.12.3.

Upgrading causes a breaking change due to prisma/tiberius#260, which affects how DATETIMEOFFSET values are inserted in SQL Server.

@jkomyno jkomyno requested a review from a team as a code owner November 5, 2024 10:38
@jkomyno jkomyno requested review from wmadden and removed request for a team November 5, 2024 10:38
@jkomyno jkomyno added this to the 6.0.0 milestone Nov 5, 2024
Copy link

codspeed-hq bot commented Nov 5, 2024

CodSpeed Performance Report

Merging #5037 will not alter performance

Comparing feat/upgrade-tiberius-0.12.3 (20d8128) with main (92f6d89)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Nov 5, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.032MiB 2.032MiB 0.000B
Postgres (gzip) 816.877KiB 816.877KiB 0.000B
Mysql 1.995MiB 1.995MiB 0.000B
Mysql (gzip) 802.582KiB 802.581KiB 1.000B
Sqlite 1.895MiB 1.895MiB 0.000B
Sqlite (gzip) 763.336KiB 763.335KiB 1.000B

@jkomyno jkomyno removed the request for review from wmadden November 5, 2024 10:52
@aqrln
Copy link
Member

aqrln commented Nov 7, 2024

The way DateTime values work in Prisma, any timezone information is currently stripped and time is converted to UTC, so, generally speaking, there's no way in Prisma to actually write the timezone offset to a column with @db.DateTimeOffset native type, except using raw queries with datetimes passed as strings. Although you can pass datetime values as strings in normal Prisma queries too, those will be parsed by Prisma and thus stripped of the timezone information.

Strings in raw queries bypass both this conversion and the bug in Tiberius because they are carried along as strings all the way to the database and aren't serialized as datetimes with offsets in binary form in TDS wire protocol.

Datetime values with UTC offsets were not affected by the bug in Tiberius because applying an offset of 0 twice still yields the same result.

This means that Prisma was never affected by that bug when writing data, only when reading data that was written using raw queries or using something else than Prisma. Moreover, in the case when Prisma is reading back a value stored using a raw query, it would actually read a different, wrong value, and not what user wrote in the raw query. Unlike the scenario described in the Tiberius issue, where the values are stored in the wrong format, but what the library reads back is consistent with what it wrote, here Prisma is being inconsistent with itself. Again, the reason for that is that the buggy code path is only used for reading and not for writing.

In other words, while this fix was a breaking change for other users of tiberius Rust crate, it is not a breaking change for Prisma and our users. Updating tiberius in Prisma is purely a bugfix that we could have shipped long ago without waiting for a major release.

Reproduction:

schema.prisma

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

datasource db {
  provider = "sqlserver"
  url      = env("TEST_MSSQL_JDBC_URI")
}

model A {
  id Int      @id @default(autoincrement())
  dt DateTime @db.DateTimeOffset
}

index.ts

console.log(
  await prisma.a.create({
    data: {
      dt: "2024-11-07T14:35:30.998+02:00"
    }
  })
);

console.log(
  await prisma.$queryRaw`
    insert into a (dt)
    output inserted.id, inserted.dt
    values ('2024-11-07T14:35:30.998+02:00')
  `
);

console.log(await prisma.a.findMany());

shell

$ tsx index.ts
{ id: 1, dt: 2024-11-07T12:35:30.998Z }
[ { id: 2, dt: 2024-11-07T10:35:30.998Z } ]
[
  { id: 1, dt: 2024-11-07T12:35:30.998Z },
  { id: 2, dt: 2024-11-07T10:35:30.998Z }
]

$ sqlcmd -d master -P Pr1sm4_Pr1sm4 -S localhost,1433 -U SA -Q 'select * from a'
id          dt
----------- ---------------------------------------------
          1            2024-11-07 12:35:30.9980000 +00:00
          2            2024-11-07 14:35:30.9980000 +02:00

(2 rows affected)

Explanation of the output:

We try to create the same datetime value with the time component equal to 14:35 (UTC+2): once using a create query and once using a raw query, and then fetch both of them again using findMany. In the sqlcmd output we can see that both values were stored correctly and correspond to the same point in time, the first one was just converted to UTC (as expected in Prisma). However, in the output of the script we can see that the value stored using the raw query is being read back incorrectly, with the timezone offset subtracted twice.

@aqrln aqrln changed the title feat(quaint): [breaking change] upgrade tiberius to 0.12.3 feat(quaint): upgrade tiberius to 0.12.3 Nov 7, 2024
@aqrln aqrln merged commit 2c64fd3 into main Nov 7, 2024
368 checks passed
@aqrln aqrln deleted the feat/upgrade-tiberius-0.12.3 branch November 7, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants