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

No ZonedDateTimeType #1

Open
mnavarrocarter opened this issue Apr 26, 2021 · 10 comments
Open

No ZonedDateTimeType #1

mnavarrocarter opened this issue Apr 26, 2021 · 10 comments

Comments

@mnavarrocarter
Copy link

Hey @BenMorel thank you for this package. I always use this library for my entities, and I always made the types myself. I'm going to switch to this one, so thank you a lot for making it available.

I just have one question. Why not a ZonedDateTimeType? I understand this could be due to some databases not supporting timezones in their datetime types, but we could always convert it to utc prior to storage, right?

Please correct me if I'm wrong.

@BenMorel
Copy link
Member

Hi @mnavarrocarter, thank you for your support!

Indeed, the lack of ZonedDateTimeType is due to poor support for a date-time + timezone from databases (or should I say, from MySQL, which is my main DB), so rather than doing it the wrong way, I preferred to leave it aside for now.

Some arguments:

  • I don't think converting to UTC is the right thing to do, as you may retrieve a date-time that's different from the date-time you originally stored;
  • Doctrine doesn't allow a type to map to more than 1 column, so we don't have an opportunity to store the timezone explicitly;
  • Types are static, they can't be instantiated with a constructor, so we cannot instantiate them with options such as a default timezone;
  • For databases that don't support a date-time-with-a-timezone, it's unclear whether a ZonedDateTime should be stored as an integer timestamp, or as a DATETIME.

If anything, we could provide support for databases that do support an explicit timezone (Postgres, AFAICS?), but I'm not sure if providing broken support for other databases such as MySQL is a good idea.

What database do you use? Do you have better ideas?

@mnavarrocarter
Copy link
Author

Hi @BenMorel, your comments launched me into research and I think I discovered a flaw in the way I'm doing things.

I'm using PostgreSQL. Along with Oracle they are the only two vendors that support storing dates with timezone information. But they do not store that information using a timezone name (i.e. Europe/London) but rather the the timezone offset. This may cause tricky issues with DST situations. I'm storing them like that, and I should fix that immediately, since I need the timezone name and not the offset.

Now, with that in mind, I have some thoughts on your arguments.

  • Changing timezone is a display concern, at least for me. If you change the timezone of a date time, the UNIX epoch remains the same. So even when the value object is being modified, the internal representation is the same. So, getting a different date-time object is not really an issue if represents the right Instant. Moreover, you'll have the same problem from the client side: if I instantiate a date-time in, say, Denver\Colorado, when I store it using a type not aware of the timezone, I'll loose the timezone information related to that date. Next time I fetch it, the Instant will be different.
  • What you say here is correct and there is no way around it.
  • What I'm suggesting is not a default option. ZonedDateTimes will always be casted to UTC as it is an absolute representation.
  • This is the core of my proposal. For all databases we should store ZonedDateTime as UTC. The Instant will be the same, the formatting is given by the AbstractPlatform. We could give a warning that this will happen using this type in order to normalise the representation. It is up to the client code to cast to the needed timezone.

I think this makes sense in my head, but I'm very opened to discussion on potential issues of this approach or corrections of my understanding. I'm by no means an expert, but I have some experience maintaining a multi-tenant booking application for a US client. We display dates in the client timezone (that we obtain from configuration). Everything in UTC is a rule that has worked great for us. No confusions, since UTC is sort of an absolute representation.

Thanks so much Ben for taking the time to reply. I know you maintain a lot of packages and that must be very busy. I just want to add on a personal note that I've followed your work from a couple of years now and I admire you for it. Your packages are of a tremendous quality. Hope we can meet in person at a conference one day!

@BenMorel
Copy link
Member

Hi Matias,

I really have a problem with ZonedDateTime swapping timezones on persist/reload. If your entity has a ZonedDateTime, that means that it does not only care about the Instant, but also about a date and time somewhere on the globe. IMO, if what you care about is the Instant, then your entity should store an Instant, and convert to a ZonedDateTime explicitly when required with Instant::atTimeZone().

In the current state of things, and unless we're storing the ZonedDateTime as a VARCHAR, I really think that entities should only store an Instant or a LocalDateTime (I do have a problem with the fact that they, also, can lose precision (nanos) on persist/reload though!)

If we want to support ZonedDateTime, IMO we should only do it for Postgres and Oracle then; and with a big warning that the timezone region will be lost on persist, and that only the offset will be retained. Or, just throw an exception if the ZonedDateTime's timezone is a TimeZoneRegion and not a TimeZoneOffset (better, IMO).

Thanks so much Ben for taking the time to reply. I know you maintain a lot of packages and that must be very busy. I just want to add on a personal note that I've followed your work from a couple of years now and I admire you for it. Your packages are of a tremendous quality. Hope we can meet in person at a conference one day!

It really makes me happy to know that my packages are useful to people, so thanks a lot for your kind words; it'd be a pleasure to meet in person!

@mnavarrocarter
Copy link
Author

mnavarrocarter commented Apr 27, 2021

Hi Ben

I think the ZonedDateTime support for Postgres and Oracle is a good compromise. This is what I currently do anyways.

doctrine/dbal does it this way. They composed a warning in the type documentation, and also fetch the platform declaration for timezone types.

@BenMorel
Copy link
Member

Hi, first of all thanks a lot for sponsoring me, it means a lot to me! 🎉

I played a bit with PostgreSQL's TIMESTAMP WITH TIME ZONE type, but it looks like it doesn't store the timezone, but instead converts it to UTC and retrieves it as UTC as well:

CREATE TABLE x (
  ts TIMESTAMP WITH TIME ZONE
);
  
INSERT INTO x VALUES('2021-04-28 23:13:00+00');
INSERT INTO x VALUES('2021-04-28 23:13:00+02');
  
SELECT * FROM x;
2021-04-28T23:13:00Z
2021-04-28T21:13:00Z

Did I miss something?

@mnavarrocarter
Copy link
Author

Hi, first of all thanks a lot for sponsoring me, it means a lot to me! tada

You are most welcome!

Oh, I'll check with my Pg setup and get back to you.

@ste93cry
Copy link

I'm evaluating to switch to the brick/date-time lib and I'm having an hard time understanding some things: I'm using MySQL, which supports either the TIMESTAMP or the DATETIME column types. Looking at the code of this package, the Instant is mapped back to INT while LocalDateTime is mapped back to the usual DATETIME type. I understand that the latter class has no knowledge of the timezone, however what I don't understand is what would be wrong with storing ZonedDateTime(UTC) into a DATETIME field. As far as I understand, an Instant and a ZonedDateTime(UTC) are the same thing. I also wonder what I should use on the application side to represent something like a creation date: as of now, I store the value in a DATETIME field, therefore it would be converted to a LocalDateTime according to this package: however, since this is a fixed point of the timeline I would expect back either an Instant or a ZonedDateTime

@mnavarrocarter
Copy link
Author

mnavarrocarter commented Jul 21, 2021

I think I might shed some light here.

It is my understanding that when any piece of software represents time, it does so by using the unix time since the epoch, so an uint32 or uint64, depending of the implementation. This is true of database systems also: they may offer a DATETIME type for allowing the use of some special functions in it, but internally, when stored, I'm quite certain it stores it as an integer (in its binary form).

The Instant (seconds since the epoch) is the absolute time. Timezone information is just a presentation concern. For instance, changing the timezone of a ZonedDateTime does not modify the Instant of that that object: it still is the same unix epoch. What it does modify is how that object may be presented. At the same instant, it could be 3 PM in Chile but 8 PM in the UK. Days, months, hours, minutes and seconds are just a presentation concern.

I hope that helped. :)

@ste93cry
Copy link

ste93cry commented Jul 21, 2021

That's clear. In MySQL, a value stored in a TIMESTAMP is converted to UTC before saving while any value saved into a DATETIME field is kept as-is and no timezone conversion happens at all. So you could say that if you don't do the conversion yourself before saving a DATETIME you may save a date and retrieve another one if in the meanwhile the application changed the timezone

In the current state of things, and unless we're storing the ZonedDateTime as a VARCHAR, I really think that entities should only store an Instant or a LocalDateTime (I do have a problem with the fact that they, also, can lose precision (nanos) on persist/reload though!)

Regarding this, I don't understand why an Instant is stored as a TIMESTAMP and not as a DATETIME (the UTC conversion is done in the Doctrine type). In fact, both types can store dates with a precision of up to microseconds (6 digits) precision, with the difference that the latter is also able to store dates after the year 2038. Will I have issues if I store the Instant into a DATETIME?

@bendavies
Copy link

bendavies commented Nov 4, 2021

Hi, first of all thanks a lot for sponsoring me, it means a lot to me! 🎉

I played a bit with PostgreSQL's TIMESTAMP WITH TIME ZONE type, but it looks like it doesn't store the timezone, but instead converts it to UTC and retrieves it as UTC as well:

CREATE TABLE x (
  ts TIMESTAMP WITH TIME ZONE
);
  
INSERT INTO x VALUES('2021-04-28 23:13:00+00');
INSERT INTO x VALUES('2021-04-28 23:13:00+02');
  
SELECT * FROM x;
2021-04-28T23:13:00Z
2021-04-28T21:13:00Z

Did I miss something?

@BenMorel probably.

To quote the docs:

All timezone-aware dates and times are stored internally in UTC. They are converted to local time in the zone specified by the TimeZone configuration parameter before being displayed to the client.

you can SHOW TIMEZONE to see what yours was.

Hopefully this demonstrates it:

CREATE TABLE x (
    ts TIMESTAMP WITHOUT TIME ZONE,
    tstz TIMESTAMP WITH TIME ZONE
);

SET timezone = 'Europe/London';

INSERT INTO x (ts, tstz)
VALUES('2021-04-28 23:13:00-04','2021-04-28 23:13:00-04');
INSERT 0 1

TABLE x;
         ts          |          tstz
---------------------+------------------------
 2021-04-28 23:13:00 | 2021-04-29 04:13:00+01
(1 row)

SET timezone = 'America/New_York';
TABLE x;
         ts          |          tstz
---------------------+------------------------
 2021-04-28 23:13:00 | 2021-04-28 23:13:00-04
(1 row)

SET timezone = 'America/Los_Angeles';
TABLE x;
         ts          |          tstz
---------------------+------------------------
 2021-04-28 23:13:00 | 2021-04-28 20:13:00-07
(1 row)

Since postgres stores the value in UTC (and not the original offset) it's impossible to guarantee that inserting a ZonedDateTimeType and then selecting back into a ZonedDateTimeType will result in identical ZonedDateTimeTypes, since it'll depend on the current timezone setting.

if this is acceptable then you could implement ZonedDateTimeType with that caveat, but this is also how doctrines DateTimeTzType works, with the same caveat.

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

No branches or pull requests

4 participants