drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
23.72k stars 587 forks source link

[FEATURE]: "Transparent" Mode for Datetime Handling compatible with the underlying driver (timezone) #1626

Open FredericLatour opened 10 months ago

FredericLatour commented 10 months ago

Describe what you want

Context

I've been working with Drizzle for a possible adoption and have noticed that Drizzle is manipulating date values in a way that doesn't align with the behavior of the underlying MySql2 driver or other ORM/query builders that rely on it.

Indeed, MySql2 makes it possible to configure the timezone at the connection/pool level (other drivers, at least in MySql space offer a similar option): Connection Options

This timezone configuration option makes it possible to manage scenarios where dates in a MySQL database are not stored in the UTC format (which is more frequent that one may imagine). Essentially, this option ensures that all dates being sent to MySQL are converted from JavaScript's internal UTC format to the configured timezone. Similarly, all dates retrieved from MySQL are first converted back to UTC, before being transformed into JavaScript date objects. This is crucial because the JavaScript Date constructor expects an input string to be in UTC format. Therefore, this feature ensures seamless conversion and compatibility between MySQL and JavaScript date formats, regardless of the timezone configuration.

Problem

I've conducted several tests attempting to modify the timezone configuration during the creation of a MySql2 connection. However, these changes didn't seem to have any effect. Without having looked into the source code, it appears as though this option is being bypassed or short-circuited by Drizzle.

In my opinion, it would have been preferable if Drizzle didn't manipulate dates and leave the driver data untouched. Many SQL drivers, including MySql2, already have a built-in solution for handling timezones.

A Universally Appealing Proposed Solution :)

That being said, given Drizzle's current approach to date handling, I believe a potential solution could be the introduction of a "transparent" mode along the "date" and "string" modes. This mode would allow users to opt out of Drizzle's date manipulation and rely on the underlying driver's date handling instead.

This new mode would provide a non breaking soloution that would make it possible to adopt drizzle in a smooth fashion for those having existing apps/database that rely on non utc stored dates.

Hopefully, this is a straigthforward change that will be accepted.

FredericLatour commented 10 months ago

Hi, I made some additional testing on my side:

Basically, if I map the date column the following way, it will revert to driver default behavior and comply to the connection timezone configuration:

testdate.date_mode_date.mapFromDriverValue = (value: any) => new Date(value)
testdate.date_mode_date.mapToDriverValue = (value: any) =>  value

With that in mind, I believe that adding the "transparent" or "driver" mode only involves the following changes in `mysql-core/columns/datetime.ts" file :

image

FredericLatour commented 10 months ago

I would be pleased to provide a PR however I'm not sure how the tests are organized. Is there some documentation on the overall structure of the code and testing approach ?

Angelelz commented 9 months ago

I don't this will be necessary because we'll start bypassing the driver's default behavior. Which means transparent mode will be equivalent to string mode. See #1659

Edit: If the way I did it in that PR is approved, you can probably take a look at the implementation there and submit one for mysql2 or others.

FredericLatour commented 9 months ago

I don't know much about Postgres and am not sure if you are facing the same problem as with MySql (could be). It does not seem that your change introduces a new mode or am I missing something? How do you avoid a breaking change? Not sure to understand what you mean by transparent (well, I ended up calling it 'driver') mode will be equivalent to string mode ? The driver (or transparent) mode will return a type date and expect a type date. If you provide a local date in string mode, it will have the same effect as fact as db storage is concerned and granted that connection timezone is configured to translate to local on the server, but the similarity stops there. Or maybe there is something that I don't understand?

Angelelz commented 9 months ago

On that PR, I basically disabled any mapping made by the driver. That means, that whatever is in the database is what drizzle will receive.

MySql is a little different in the sense that it doesn't store timezone information. So Drizzle will offer a string mode, in which it will return the string that it got from the database, without any manipulation/mapping, making it a driver mode. In Date mode, drizzle would just assume that the Date coming is in UTC and parse it as such into a Date object.

I guess we shall see if the drizzle team likes that approach, then you could submit a PR with those changes. I could do it too.

FredericLatour commented 9 months ago

driver

I have the feeling that by driver you mean "Drizzle"? When I'm using the term "driver" I mean the underlying driver (ie: MySql2)?

"string" format

The problem is that, even though it looks similar at first sight, it is not the same thing to have the date in string format. You don't have any function to convert a string local date to a date. New Date(some_date) expects a date string in UTC format unless you provide timezone information. Therefore you'll need to make the necessary adjustment to convert the "string" date into date. This string mode does not really make sense if you ask me. To be honest, I don't really understand why Drizzle team went to any date manipulation instead of relying on the driver.

your pr

Correct me if I'm wrong but it looks to me like your PR constitutes a breaking change. it seems to me more reasonable to introduce a new mode called "driver" or "transparent".

Angelelz commented 9 months ago

No, by driver I mean (in your case) Mysql2. new Date(some_date) can parse many types of dates, including postgres format and mysql format as well. For mysql format, you could do new Date(some_date_string + 'Z') to force it to be parsed as UTC. Which what I did in my PR.

I think the way I'm handling dates makes strings only necessary when microsecond precision is necessary (Date objects are not that precise).

My PR is not breaking changes, is fixing a lot of reported bugs.

The idea of drizzle manipulating the types for you makes a lot of sense, after all it's an Object Relational Mapper.

FredericLatour commented 9 months ago

Having to convert dates into strings before updating your database and strings into dates after retrieving data from a database is not really a great experience. Moreover, if you want to store the date in local format, you have to make your own conversion. There is no function to convert a date into pseudo-local iso string format.

How can there be a lot of reported bugs on date handling? If they are that many bugs related to dates, people have certainly found a workaround. In that respect, fixing bugs sometimes involves breaking changes. I don't know if there is something specific to Postgres but I can't see how I could have the dates being handled in compliance with the timezone defined at the connection level without offering an additional mode. I don't understand how you would do that without introducing a breaking change?

We will have to disagree here. The idea of drizzle manipulating the values returned by the underlying driver does not make any sense unless for additional optional features. Those drivers let you already query SQL data and have already a solution to those timezone problems. Forcing you into storing date into UTC (or deal with the string format) is not an additional feature. On the contrary, the underlying driver can already do this and more. On the other hand, the string format can indeed be considered as an additional feature for those who would like for some reason deal with date in string format (serialization with mostly no manipulation).

Angelelz commented 9 months ago

The mode: 'date' would do everything you're looking for. If you configure your timezone by session/connection, that will also be respected. What I'm saying here, is that drizzle currently has a bug that led you to your research and the subsequent opening of this issue. If drizzle was handling dates in mysql like my PR will do in postgres, we wouldn't be having this conversation.

FredericLatour commented 9 months ago

image

Of course, I would be fine with that change but, because it was clearly said (see screenshot above) that UTC was the behavior of the date mode, changing the behavior would constitute a breaking change in my book. With the current version of Drizzle, whatever your timezone configuration, the date mode will store dates in utc in MySql. If you correct the behavior, it constitutes a breaking change because I could now have a different value depending of the timezone configuration. In most cases, it should not imply any change as the default driver behavior is utc. However someone could have configured some timezone, and because it was not working as he expected, found a workaround. Depending on his workaround, he could suddenly have different results with such a change.

I have nothing against a Breaking change. This is certainly more elegant than having a third mode. However it should be clearly explained and documented.

Angelelz commented 9 months ago

Yes, I implemented that PR. Now I'm curious, are you using datetime or timestamp? I was assuming we were not doing anything to the types but we are. What's the problem that you're facing with the types?

Angelelz commented 9 months ago

I just ran a quick test in a mysql database:

const table = mysqlTable("test_Dates", {
  id: serial("id").primaryKey(),
  date1: datetime("date1", { mode: "date", fsp: 3 }).default(
    sql`current_timestamp`,
  ),
  date2: datetime("date2", { mode: "string", fsp: 3 }).default(
    sql`current_timestamp`,
  ),
  date3: timestamp("date3", { mode: "date", fsp: 3 }).default(
    sql`current_timestamp`,
  ),
  date4: timestamp("date4", { mode: "string", fsp: 3 }).default(
    sql`current_timestamp`,
  ),
});

const parse1 = table.date1.mapFromDriverValue;
const parse2 = table.date2.mapFromDriverValue;
const parse3 = table.date3.mapFromDriverValue;
const parse4 = table.date4.mapFromDriverValue;

table.date1.mapFromDriverValue = (val: any) => {
  console.log(val);
  return parse1(val);
};
table.date2.mapFromDriverValue = (val: any) => {
  console.log(val);
  return parse2(val);
};
table.date3.mapFromDriverValue = (val: any) => {
  console.log(val);
  return parse3(val);
};
table.date4.mapFromDriverValue = (val: any) => {
  console.log(val);
  return parse4(val);
};

const created = await db.execute(sql`CREATE TABLE IF NOT EXISTS ${table} (
  id serial primary key,
  date1 datetime(3) default current_timestamp(3),
  date2 datetime(3) default current_timestamp(3),
  date3 timestamp(3) default current_timestamp(3),
  date4 timestamp(3) default current_timestamp(3)
)`);

console.log(created);

const inserted = await db.insert(table).values({});

console.log(inserted);

console.log(await db.select().from(table));

await db.execute(sql`drop table if exists ${table}`);

I got correct results:

[
  {
    id: 1,
    date1: 2023-12-17T05:18:26.446Z,
    date2: "2023-12-17 05:18:26.446",
    date3: 2023-12-17T05:18:26.446Z,
    date4: "2023-12-17 05:18:26.446",
  }
]
FredericLatour commented 9 months ago

What timezone did you configure at the connection level ? What does the data look like in your database ?

I had not paid that much attention to your example but you don't even write in the database which only displays half of the story.

But still, write some data even manually in the mySql table, set the timezone at the connection level (ie: timezone: "+03:00") and compare the date retrieved by a mysql2 query and a drizzle query. The timezone configuration tells MySql2 that the dates in the MySql database accessed are stored in this timezone (+3). Therefore it will convert back and forth to satisfy the configuration. Does it make sense? I know those timezone stuff are really tricky and that's why drizzle should have let it go and rely on the driver.

I will try to come up with a prog that highlights the problem.

Angelelz commented 9 months ago

Changing the timezone would complicate you even more IMO, do you apply the change when writing, reading, both? The current behavior is to write all dates to the DB in UTC. Since that's how they're represented inside a Date object, the conversion is trivial. Also, datetime in MySql doesn't store any information of the timezone, making it more error prone to attempt to store any other timezone other than UTC. I adapted my example and inserted a specific date:

const exampleDate = new Date("2002-03-23T10:11:12.123Z");

const inserted = await db.insert(table).values({
  date1: exampleDate,
  date2: exampleDate.toISOString().replace("T", " ").replace("Z", ""),
  date3: exampleDate,
  date4: exampleDate.toISOString().replace("T", " ").replace("Z", ""),
});

console.log(inserted);

console.log(await db.select().from(table));

And this was the result, which is correct:

[
  {
    id: 1,
    date1: 2002-03-23T10:11:12.123Z,
    date2: "2002-03-23 10:11:12.123",
    date3: 2002-03-23T10:11:12.123Z,
    date4: "2002-03-23 10:11:12.123",
  }
]
FredericLatour commented 9 months ago

Starting from your program, I made a couple of changes. The goal of the program is to insert and then select dates with both Drizzle and MySql2 with some specific timezone configuration. I have commented the program and it should be straightforward to follow. I will comment results in an additional post.

import { sql } from "drizzle-orm"
import { mysqlTable, int, varchar, datetime, serial, timestamp } from "drizzle-orm/mysql-core"
import { drizzle } from "drizzle-orm/mysql2"

import mysql from "mysql2/promise"

// If you don't want to configure any timezone and rely on default driver behavior,
// just set connectionTimezone to an empty object
const connectionTimezone = {timezone: getTimeZone()}
const connection = await mysql.createConnection({ uri: process.env.DATABASE_URL, ...connectionTimezone })
const db = drizzle(connection)

const table = mysqlTable("test_Dates", {
  id: serial("id").primaryKey(),
  description: varchar("description", { length: 255 }),
  date1: datetime("date1", { mode: "date", fsp: 3 }).default(sql`current_timestamp`),
  date2: datetime("date2", { mode: "string", fsp: 3 }).default(sql`current_timestamp`),
  date3: timestamp("date3", { mode: "date", fsp: 3 }).default(sql`current_timestamp`),
  date4: timestamp("date4", { mode: "string", fsp: 3 }).default(sql`current_timestamp`),
})

// drop the table at start so that we can check the table after each run
await db.execute(sql`drop table if exists ${table}`)

const created = await db.execute(sql`CREATE TABLE IF NOT EXISTS ${table} (
    id serial primary key,
    description varchar(255),
    date1 datetime(3) default current_timestamp(3),
    date2 datetime(3) default current_timestamp(3),
    date3 timestamp(3) default current_timestamp(3),
    date4 timestamp(3) default current_timestamp(3)
  )`)

// Current datetime
const myDate = new Date()
const myDateStr = toLocaleISOString(myDate)

// Insert a row using Drizzle
const inserted = await db
  .insert(table)
  .values({ description: "inserted with drizzle", date1: myDate, date2: myDateStr, date3: myDate, date4: myDateStr })

// Insert a row directly with mysql2
connection.execute(`insert into test_Dates (description, date1, date2, date3, date4) values (?, ?, ?, ?, ?)`, [ "inserted with mysql2", myDate, myDateStr, myDate, myDateStr])

console.log('Info')
console.log({ current_timezone: getTimeZone(), connectionTimezone, myDate, myDateStr })

console.log("Select with Drizzle")
console.log(await db.select().from(table))

console.log("select with raw sql")
const rows = await connection.query(`select * from test_Dates`) 
console.log(rows[0])

/**
 * Converts a Date object to a localized ISO string representation.
 * @param date - The Date object to convert.
 * @returns The localized ISO string representation of the Date object.
 */
function toLocaleISOString(date: Date) {
  const pad = (num: number) => num.toString().padStart(2, "0")

  const year = date.getFullYear()
  const month = pad(date.getMonth() + 1) // Months are 0-indexed in JavaScript
  const day = pad(date.getDate())
  const hours = pad(date.getHours())
  const minutes = pad(date.getMinutes())
  const seconds = pad(date.getSeconds())

  return `${year}-${month}-${day}T${hours}:${minutes}:${seconds}`
}

/**
 * Returns the current time zone offset in the format "+HH:MM" or "-HH:MM".
 * @returns {string} The current time zone offset.
 */
function getTimeZone() {
  const offset = new Date().getTimezoneOffset(),
    o = Math.abs(offset)
  return (
    (offset < 0 ? "+" : "-") +
    ("00" + Math.floor(o / 60)).slice(-2) +
    ":" +
    ("00" + (o % 60)).slice(-2)
  )
}
Angelelz commented 9 months ago

Let me run your example and share some thoughts

FredericLatour commented 9 months ago

Let's run the program as if I was in Los Angeles and AS IF I want to store dates in local timezone format in my database.

TZ='America/Los_Angeles' bun run src/drizzle/testdate.mts

The resulting data is the following:

Info
{
  current_timezone: "-08:00",
  connectionTimezone: {
    timezone: "-08:00",
  },
  myDate: 2023-12-17T13:24:48.478Z,
  myDateStr: "2023-12-17T05:24:48",
}
Select with Drizzle
[
  {
    id: 1,
    description: "inserted with drizzle",
    date1: 2023-12-17T13:24:48.478Z,
    date2: "2023-12-17 05:24:48.000",
    date3: 2023-12-17T13:24:48.478Z,
    date4: "2023-12-17 05:24:48.000",
  }, {
    id: 2,
    description: "inserted with mysql2",
    date1: 2023-12-17T05:24:48.478Z,
    date2: "2023-12-17 05:24:48.000",
    date3: 2023-12-17T05:24:48.478Z,
    date4: "2023-12-17 05:24:48.000",
  }
]
select with raw sql
[
  {
    id: 1,
    description: "inserted with drizzle",
    date1: 2023-12-17T21:24:48.478Z,
    date2: 2023-12-17T13:24:48.000Z,
    date3: 2023-12-17T21:24:48.478Z,
    date4: 2023-12-17T13:24:48.000Z,
  }, {
    id: 2,
    description: "inserted with mysql2",
    date1: 2023-12-17T13:24:48.478Z,
    date2: 2023-12-17T13:24:48.000Z,
    date3: 2023-12-17T13:24:48.478Z,
    date4: 2023-12-17T13:24:48.000Z,
  }
]

And here is a screenshot of the table data in the database: image

So, what's going on here ?

Simple, mysql2 driver will convert back and forth the dates using the connection timezone configured. Therefore, I will get my dates in my local timezone stored in my database. Drizzle will force me into UTC for the date type. It may look like it's working properly if you don't pay too much attention. But that's not what I want. Does it clarify?

FredericLatour commented 9 months ago

With how drizzle currently works, people having a database where dates are not stored in UTC (whether this is a good thing or not) have the following options:

I believe Drizzle current way is not correct but still works as long as you are having dates in UTC format in your database. That's why I proposed to add an additional "mode" to avoid any breaking change while proposing what I consider the proper approach for dealing with timezone without forcing you to store dates in UTC.

Angelelz commented 9 months ago

I am currently maintaining the timeclock app in my company, the app is used to track the employee's hours.

Our field technicians have to go out of state and into different timezones sometimes. In our opinion, managing timezone per connections would have had so many foot guns, especially because the server will maintain a pool of connections, we decided to keep the connection in UTC and manage the timezone changes ourselves. It is surprisingly easy, we have this one helper in the codebase that we use in the client, both in the web app and the mobile apps:

export function removeTimezone(
  time: Date,
  offsetHrs: number = new Date().getTimezoneOffset() * 60 * 1000
) {
  return new Date(time.valueOf() - offsetHrs);
}

I submitted the UTC PR because it would represent an unique/centralized way of storing dates in Mysql (taking into account that MySql doesn't store timezone information).

Without sounding pedantic (hopefully), and without knowing your application's requirements/features, you could envision in your application a similar helper, where you give it the offset and it would store the date in your datebase with the offset that you please. And you'd be working with Dates the whole time (as we do).

You've also noticed that I haven't said that drizzle shouldn't support this. My opinion is that it shouldn't, but it will be the drizzle team to decide.

FredericLatour commented 9 months ago

Why would I do anything like what you are suggesting when the driver works perfectly well perfectly for the scenario you are describing ant the one I am describing as well?

The Timezone configuration tells the driver in which Timezone dates are stored in the database. Period. Locally, dates are handled in UTC anyway (JS Date object). You want to store dates in UTC in the database. No problem, do not use any timezone configuration at the connection level.

I'm rather surprised by your opinion that Drizzle should not support a proven approach to timezone handling (on a technical standpoint - functional issues are something else) . Typeorm relies on Mysql2, Kysely relies on MySql2, like Knex and most tools on top of MySql2. Other drivers like maria implements this same timezone approach as well.

Unless you are saying storing dates in a format different than UTC is WRONG and people using my tool should not be able to do that, I can't see why Drizzle would diverge and be incompatible with all those tools. Honestly, it does not make sense to me.

Angelelz commented 9 months ago

That is just my opinion. In any case, it seems like there will be an option to add an offset to the options per column. See this comment on my PR.

Edit: I didn't see an issue tracking that, not sure if you'll like to submit it?

FredericLatour commented 9 months ago

Of course, all opinions are respectable, but I'd be lying if I didn't say I find it completely illogical. I can change my mind at anytime if I'm wrong or presented with valid arguments. However, unless I've missed something, you can store dates in UTC by letting the underlying driver doing its things without any timezone configuration. That's the default behavior. Either I'm right or wrong. And if I'm right, the current date manipulation in Drizzle does not make sense and at the same time makes it difficult to store date in non UTC timezone.

Honestly, I don't think adding an offset is a good idea. MySql2 is already doing this. I can't see any valuable reason to do, another way, exactly the job that the underlying driver is doing. Most ORM and Query Builders on top of MySql2 are just doing that. And it works. One could argue that by doing this, it will make up for the initial design error in a more elegant way than adding a third mode. Not completely wrong. However, I'm not really fond of having even more date manipulation on top of the current one.

My proposal is plain simple a "driver" or "transparent" mode that let's the driver do it's things. That would ensure that if you are coming from mysql2, typeorm, knex and many other tools that you won't face any weird date problems.

This is not the most elegant approach to have 3 modes but the best approach for not breaking compatibility. I'm afraid that date handling was taken the wrong way from the very beginning. There should have been good reason to change the default driver handling. I can't see any.

I'm talking of MySql only. I don't know how dates in Postgres are working and I don't know how the drivers in PG ecosystem usually handle TZ conversion. I heard that PG dates include tz information. It must certainly change things. However, it would be surprising if they were not already proposing a valid solution.

I'm not part of the Drizzle team. But if I were, I would take time to reason about date handling, if they took it the wrong way or not and maybe rethink the approach. I can't see why all the other tools would be wrong.

Angelelz commented 9 months ago

Alright, I'll explain why I don't think the approach you suggest is the best. Now, just please remember that this is just some guy-on-the-internet's opinion, IDK what would the drizzle team think about this.

An offset configuration on the column, would solve all the problems, it would give you the flexibility to change your setting per column and will make sure your types are not lying to you.

This problem is not only present for dates, you simply cannot properly represent some mysql fixed-point types (like decimal) in javascript due to the inherit flaws of floating point numbers. Some drivers could give you back the strings, others could just give you a number. Then there are bigints, some drivers might give you strings, others might give you number and others might give you js Bigints. An user should, per column, be able to select what typescript type he'll like, and this is only possible if the ORM does the mapping. That's what the M stands for.

FredericLatour commented 9 months ago

Yes, no problem. Your comments are well taken. Those are interesting points but let me answer to them:

  1. I have no experience with Planetscale but as far as I can see, I can use MySql2 with it. If I use MySql2, the timezone will be applied as expected. If I'm using another specific driver (planetscale?), the driver specific behavior will be expected if I was using the mode driver (if it existed). I don't understand your reasoning about a mode mysql2 | planetscale. driver means whatever the driver does - take it or leave it. That being said, in most cases the driver has already a thought approach for handling dates.

  2. if the planetscale user uses mysql2 driver, and unless I'm missing something, there won't be any difference in development with a local mysql database. Moreover, I will add the following points:

    1. planetscale diverges from MySql in many ways, therefore, a planetscale user is plenty aware of taking care of those incompatibilities.
    2. As far as I could read, the development story and experience with planet scale is supposed to be online with branching and such. The planetscale serverless driver will not work locally anyway so that it's not even relevant.
  3. I don't get your point here. You are relying on the driver to send and retrieve the data. If you want to handle everything from start to end, you can do it the Prisma way. They are not using an existing driver so that there is no specific expectation (and by the way, they have it wrong on the timezone side, it's been 5 years that some frustrated users are expecting it to be solved - I abandoned the idea of using Prisma because of that). When you are relying on a driver, it is completely unexpected that that the SQL query built by the ORM/Query builder does not return the same data as the same query sent directly by the driver. Even more so with a tool like Drizzle which is a light orm and in many respect a sophisticated query builder.

  4. That's not only MySql2 but Mariadb driver as well. I completely don't understand why you say "I'm sure they had their reason". The reason they have is obvious. You need this configuration option in order to store dates in the timezone that best suits your app or business requirements. Of course, its not per column. Who would want a different timezone per column. On drizzle the configuration is on a per column basis so that there is no other choice. However someone that would choose transparent or driver will obviously apply it to all date columns in a Drizzle schema.

The problem of an offset configuration is that it does not solve the problem of DST. How do you handle the fact that part of the year you are GMT+1 and another part or the year, GMT+2 ? It's not possible with an offset. That's what the "local" mode is for when you use MySql2 configuration. If you really want to go that route, why not having the same approach as the timezone configuration, a new option: datemode = 'Z' | 'Local' | offset. 'Z' (meaning UTC) would be the default for avoiding any breaking change. What do you think?

By the way, I made a mistake in my previous posts, 'local' is the default MySql2 driver option and not 'Z' (UTC).

Regarding the last part of you post, I'm not sure exactly what you mean. All the drivers I have been using are providing data that I can use in the context of typescript. There is a difference between having a string or a numeric and having a different value than what you get when using the driver directly with the same query.
In my book, M stands for mapping Relational into Object and not really for transforming data. If not, The underlying driver is also an ORM :) Thad being said, "date handling" is always a mines field.

Angelelz commented 9 months ago

You can use mysql2 if you want to connect to planescale, but if you're in a serverless environment, you'll need to use the planetscale severless driver. That driver will return your dates as a string. What I'm trying to say here, is that with a transparent mode drizzle can't just give you whatever the driver returns, it needs to map the type correctly. So, now your columns need to be aware of the driver you are using to report the correct type. That's my reasoning around mode mysql2 | planetscale.

I'm not saying it's impossible, I just don't think it should be the way to solve this problem.

Just because the driver had to implement mapping to a Date object, probably because their users were asking for the ergonomics, doesn't mean it's an ORM. Keep in mind the timezone configuration they have is only needed to create a javascript date object, because mysql does not know anything about timezones, it doesn't store that information.

Anyway, Let's just see what they say.

FredericLatour commented 9 months ago

Yes, you are right regarding the necessity to map the driver value to a certain type.

That being said, if the driver returns a string do whatever is fine to provide the additional possibility to get it directly as a date object. If the driver returns a string do a best effort (I have no specific expectations anyway). If the driver returns a date, do not change it's value,

If the driver does not provide a date by itself, it means I have anyway an internal approach to handle dates. And if I move to Drizzle, I can anyway create a custom column to implement my internal approach. When the driver provides a date and a configuration option, it's completely messy to do another way. Suddenly, you application based on Typeorm, MySql2, Kysely and many others don't work anymore (Not completely true in the sense you can always create a custom column but that's weird to have to do it).

Yes, MySql does not store Timezone information with dates. That's why the Timezone configuration is an essential feature if you map MySql dates into JS dates and vice-versa. Either you decide to just return string without any interpretation or you need to provide a way to configure the Timezone.

Implementing an offset is not enough. You will need to restart your application everyday to take the DST into account. If they want to the offset way, they should provide a "local" option that will apply the proper offset depending on the DST.

FredericLatour commented 6 months ago

Any update on this matter ?

I'm currently using this approach on each table to work around the problem. It's somewhat ugly though:

;[table.created, table.modified, ].forEach((col) => {
  col.mapFromDriverValue = (value: any) => new Date(value)
  col.mapToDriverValue = (value: any) => value
})
mlev commented 2 weeks ago

For us the above workaround isn’t enough - it will only work when the local timezone matches the db timezone. Our backend runs on UTC but the DB is Sydney time (for legacy reasons). Drizzle only gets the raw string back from the driver so calling new Date() will coerce into the local timezone. i.e. the result of new Date("2024-04-05 06:07:08") will vary based on local timezone.

We’ve created a custom type that reads/writes the time into the expected timezone (using @date-fns/tz). It’s a bit of a gotcha going forward to remember to never to use the standard datetime type but can’t think of anything better right now.

FredericLatour commented 2 weeks ago

@mlev Yes, but the date returned will depend on the driver TZ configuration. Can't you configure the timezone at this (driver) level? How were you handling things without Drizzle ?

@Angelelz Why not implementing the requested "transparent" mode for those who would like the dates to be retuned exactly like they would be with a SQL query made using MySql2 directly ? Seems a pretty legit request to me.

mlev commented 2 weeks ago

@FredericLatour that wasn't the case when I was testing - it seems drizzle bypasses the driver to return a string for all date types. I'm creating the connection using mysql.createPool() from mysql2. The value passed to the mapFromDriverValue was always the same string regardless of the timezone setting I passed.

I tracked it down to this code in the drizzle session: https://github.com/drizzle-team/drizzle-orm/blob/c8359a16fff4b05aff09445edd63fc65a7430ce9/drizzle-orm/src/mysql2/session.ts#L76-L78

If I commented out that line then the value passed to mapFromDriverValue was a Date and the driver timezone setting did apply.