Bug 37952 - Store datetime DB columns in UTC
Summary: Store datetime DB columns in UTC
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 37902
Blocks:
  Show dependency treegraph
 
Reported: 2024-09-18 07:35 UTC by Jonathan Druart
Modified: 2024-10-30 02:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-09-18 07:35:31 UTC
We need to store the datetime in UTC in DB and apply the timezone on display only.

It will be more flexible and easier to maintain.
Comment 1 Nick Clemens (kidclamp) 2024-09-18 12:05:41 UTC
Currently we store them in server time and present the conf time on display, is that correct? What does UTC gain us over that?

This will impact reports from the end user - would it be worth having support in reports for datettime pickers that will convert entered date/times to UTC if we do this?
Comment 2 Jonathan Druart 2024-09-18 13:13:03 UTC
(In reply to Nick Clemens (kidclamp) from comment #1)
> Currently we store them in server time and present the conf time on display,
> is that correct? What does UTC gain us over that?

I will explain a bit more later.

But the idea here is to first convert to UTC the existing data, then remove the TZ at the koha config level.
That will bring us more flexibility, like having the tz defined at client level.
There would be a "default tz" defined at library level, so we could have an instance of Koha working on different timezones. End-user using Koha in a different timezone and see the correct time (instead of the server time).
We won't actually care about the timezone during the calculation and the need to to convert or not depending on where we are (perl, js, etc.) You can see that I have reported few bugs in the area lately and, trust me, it's a mess everywhere.
We simply need to deal with rfc3339-formatted datetimes, send them to the client/consumer, and they will convert to whatever timezone they want (this is the idea, in practice it's not that trivial :D)
This is about sanitizing, cleaning, and doing the right thing with timezone and datetime handling.

Yesterday I've posted a video to show how it would work for the end-user: https://chat.koha-community.org/koha-community/pl/zsp4pmnkbtfq9c8qad3ptnj5jr

It will also help us to find hidden bugs (where we do not deal correctly with datetime values).

> This will impact reports from the end user - would it be worth having
> support in reports for datettime pickers that will convert entered
> date/times to UTC if we do this?

Yes, definitely, you read my mind, I have this empty commit on my working branch "TODO - reports? We don't have datetime runtime parameter"
And a lot of other TODOs :D
Comment 3 Nick Clemens (kidclamp) 2024-09-19 11:50:15 UTC
(In reply to Jonathan Druart from comment #2)
> There would be a "default tz" defined at library level, so we could have an
> instance of Koha working on different timezones. End-user using Koha in a
> different timezone and see the correct time (instead of the server time).

Ah, ok, i get it now
> 
> Yesterday I've posted a video to show how it would work for the end-user:
> https://chat.koha-community.org/koha-community/pl/zsp4pmnkbtfq9c8qad3ptnj5jr

Yes, this is nice - it is a long list, so preselecting the default from above will be 
needed
Comment 4 Jonathan Druart 2024-09-19 14:02:19 UTC
I've pushed the current (very) WIP work to a remote branch
https://gitlab.com/joubu/Koha/-/commits/bug_37952

I would like some of you to confirm we want to go into this direction and people are ready to follow (help, test, QA) me.

There is a TODO commit to read first.

No need to tell me everything is buggy, yes I know (better would be to tell me what is working).

You can test:
* background jobs list and show views
* circulation (check out using a due date)

That's the only 2 views I have tested.

I have not the time to have something better looking (more cypress tests, better commit messages), but I think it's a good start and show how it could work.

Let me know what you think about it!
Comment 5 David Cook 2024-09-27 16:17:01 UTC
(In reply to Nick Clemens (kidclamp) from comment #1)
> Currently we store them in server time and present the conf time on display,
> is that correct? 

I don't think that's how it works. If I recall correctly, the time zone configuration is used to set the time zone used in the database connection (see Koha/Database.pm) and in Perl DateTime objects.

So if you've set Australia/Sydney in your koha-conf.xml, we create Perl DateTime objects according to the Australia/Sydney time zone, and then those basically get stored as strings in the MySQL database. When we retrieve those strings, we assume they're in Australia/Sydney and go from there. 

I think a sort of exception is the TIMESTAMP data type. When you're storing a TIMESTAMP, it converts from the connection's TZ into UTC for storage under the hood. If you were to connect with a different TZ, then you'd get that TIMESTAMP automagically in your connection's TZ. 

I don't 100% understand the ins and outs of MySQL datetime, as I do a lot more in this area with PostgreSQL.

> This will impact reports from the end user - would it be worth having
> support in reports for datettime pickers that will convert entered
> date/times to UTC if we do this?

This will certainly be a consideration we have to keep in mind.

Also, any SQL query that uses LIKE, because the end user is going to think they're sending a partial date using their local timezone, but as I note on bug 37902 we'll be processing it using server time.

At the moment, server time probably works  most of the time, but if/when we switch to UTC for DATE and DATETIME, we'll run into trouble where users will get unexpected results.

--

I need to do more thinking on this, review MySQL docs more, and I need to review Joubu's patches.
Comment 6 David Cook 2024-10-30 01:07:32 UTC
Taking a quick look at Joubu's branch...

22 files changed, 382 insertions(+), 138 deletions(-)

--

Previously, I hadn't thought enough about how we'd get the user's timezone. Using Javascript to set a cookie makes sense. 

Looking at the code, and I don't see anything for setting from the OPAC? Just a select menu on the staff interface?

I think it would make more sense to detect the timezone via the browser, and then set the cookie. (Probably not a bad idea to have a library/branch default in the event that there is some problem I guess...)

--

It looks like installer/data/mysql/atomicupdate/xxx.pl updates "timestamp" and "datetime" columns, "timestamp" doesn't need conversion though, since it stores in UTC behind the scenes and retrieves based on TZ in client connection. 

I'm not 100% sure how we should treat the "date" column either... 

I'll explain in my next comment...
Comment 7 David Cook 2024-10-30 01:20:24 UTC
Consider "dateofbirth" which is a "date" column in the borrowers and deletedborrowers tables.

If my date of birth is 1970-01-01 09:00 in Australia/Sydney time, that's actually 1969-12-31 22:00 UTC time.

If we just store 1970-01-01 and automatically translate it from UTC to Australia/Sydney, then 1970-01-01 00:00 becomes 1970-01-02 11:00 which is a different day. That's not going to work.

--

I suppose "dateofbirth" might be a special case where it could just always be treated as local. For instance, your date of birth is on your government identification. I'd say most authorities would treat your date of birth in terms of the local timezone.

With "volumedate" in the "biblioitems" table it would probably be a floating timezone since it would just refer to the date of publication relative to the place of publication. For argument sake, I think we could probably interpret this as local time though like "dateofbirth". 

But "dateexpiry" without a timezone would probably lead to subtle bugs for things like ./misc/cronjobs/membership_expiry.pl, since the >= and <= parameters are based off dt_from_string which will use UTC by default and execute in the context of the backend system (keeping in mind that we can have a variety of different timezones on a server for different Koha instances). Of course, maybe that means that "dateexpiry" should actually be a DATETIME value with 23:59 for the time component.

There are 97 "DATE" columns in Koha. We'd probably need to review every one of them to avoid bugs.
Comment 8 David Cook 2024-10-30 02:04:20 UTC
I think we'd need to very thoroughly test this change.

* As a first change, why don't we do some code for capturing the user's timezone, setting the cookie, and then storing it in their session? There's no harm in doing that immediately, as it would be a very simple change to make. This can be pushed as a standalone change. (In prod, I don't think we'd want to allow users to change their timezone setting in the app, but in test it would be handy to be able to change your timezone for testing the below changes...)

* A second step could be to just convert from the server TZ to the user's TZ for display. Since it's just display, bugs wouldn't have long-lasting consequences, and in many cases the converted times would probably be the same, although we could take some time to analyze this (I have a number of clients in different timezones than me). This could also be pushed following step one.

* A third step... I suppose would be converting user's TZ to server TZ for storage. (In many cases, this will probably be the same or similar time, so any bugs should be minor.)

* Fourth step would then be to convert from server TZ to UTC. (Hopefully by this point the main mechanisms have been tested and trialled in prod over time.)

--

I think that I'd feel a lot more comfortable following a plan like that. Doing everything at once scares me. 

Trying to recover from bugs with a switch to UTC will be a really difficult challenge for people like me who live in timezones at the extreme end of variation from UTC (like +11:00, +14:00, -12:00).