Refactoring how time works in Avram

[jwoertink]

This is sort of a big one, and could end up being a breaking change, so I'll try to explain the issue and possible solution.

This issue I first ran in to here was caused because I wrote a spec that inserted several records in to my DB, then sorted them by their created_at, and didn't get consistent results that I expected. This was because they were all inserted within the same second, so they all had an identical created_at. Knowing the exact order is pretty important based on time here.

On my first investigation, I noticed we don't store the milliseconds of the time. Avram just takes the time object from Crystal and calls to_s on it. So whatever format that returns is what we shove in to the DB. Changing the format here to include milliseconds fixed my issue.

Next I ran in to an issue where my specs passed locally on my Linux machine, but on my Mac they failed. I also noticed they failed on the CI. Turned out that a default postgres install takes the timezone of the host machine, and sets the timezone for that DB. My Mac used Docker for postgres, and the CI server is set to UTC, but my local Linux machine uses PDT.

This is normally ok since if you store everything locally in your local timezone, then it's all relative. Your user's created_at is in PDT, and it doesn't really matter. My issue was that I need to pull out a specific set of records within a specific time frame, and the difference between PDT and UTC is 7 hours. I was converting times to UTC in Crystal, and my local Linux converted it to PDT, but my Mac and CI kept the time as is giving me the wrong result.

To fix this, I added a migration that ensures the DB is always UTC. Then running locally at my home, or another dev that lives 3 hours ahead, it doesn't matter. We always use the same timezone, and it's all good. However, this led to another issue... Running queries that used timespan like 30.days.ago always uses Time.local. This is based on your host machine where Crystal runs, not where Postgres is at. If my DB was in PDT, and my host machine is PDT, then 30.days.ago is all the same. Now that my host machine is PDT, and postgres is UTC, it's not the same.

We can now use 30.days.ago.to_utc to get around this, but we have to ALWAYS remember to add the to_utc. Small issue, but a bit annoying.

Ok, so to recap, at this point I've

• Changed the time format that I'm inserting time records in
• Forced my DB to always use UTC no matter what
• Added extra to_utc calls all over the place to get proper queries

What's the issue now? Well, with this new format, parsing the column when pulling the value out of the DB, Avram iterates over all possible formats. If the format fails, then it moves on to the next format. What ends up happening is the first format that passes is actually %Y-%M-%d 😬 yeah... just the date... no time. Ok, fine, we can fix this by setting our own custom format. Then Avram will use this custom format each time.

These all solve the issues for my app, but in order to do this, it required me patching Avram.

The proposal

My thought is, what if Avram had a concept of a single default format, as well as a default timezone?

# config/database.cr
Avram.configure do |settings|
  settings.database_to_migrate = AppDatabase
  # These would be added
  settings.default_timezone = Time::Location::UTC
  settings.default_time_format = Time::Format::RFC_3339

  # In production, allow lazy loading (N+1).
  # In development and test, raise an error if you forget to preload associations
  settings.lazy_load_enabled = LuckyEnv.production?
end

What these would do is replace the logic of iterating over several different time formats. Maybe we get a performance boost here? It would also allow you to set a DB to some different timezone if you wanted. I've had an app in the past that required the timezone to always be local, and the app wasn't centralized. It's a rare case, but not unheard of. Right now, you're forced to always use UTC

Some questions to answer though is

• How does this affect inherited DBs (like an old rails app) where you might have a date column, and time column and datetime?
• How does this affect HTML inputs? ref
• Is there a way in Crystal to tell Time.local that it is globally a specific timezone?
• How bad will this break/affect existing Lucky apps?

[robacarp]

I like the idea of a default time format, but I think it'll need to be something that can be specified at the field level somehow on the model as well?

[jwoertink]

I was thinking about that too. I'm not sure how it would work, but I would assume that it's some optional config that uses the default when not specified.

Maybe something like:

table do
  column birthdate : Time, format: "%b %Y"
end

🤔

[jwoertink]

Think on this more, I don't think we can do it down to the column level. When we insert the value, we don't actually know which column it is. We call to_db on each value https://github.com/luckyframework/avram/blob/a3d830cac03f035a299e9c8fa6d099e43348b11b/src/avram/charms/time_extensions.cr#L74-L76 and at that level, there's no concept of "this is the created_at column, so use the to_db format created_at has set". We could do it on the SaveOperation possibly, but I'm not sure if that would cause other issues 🤔

[robacarp]

Ouch. That makes me wonder if there doesn't need to be an additional layer to the translation between database and avram, where the column declaration is made available to the to_db and from_db methods (or whatever they're called). This might not be the use case that demands it, but it seems like there might be one.

[robacarp]

I think the root cause of your issue here isn't a time formatting problem, but a drift between what the computer that's running your server think is "now" and what the database server thinks is "now". As you said, when everything is local it works fine and when everything is UTC it works fine. In the case of the Postgres docker image, they've run into this issue before and provide some environment variables to solve the sync issue.

Is there some room in the boot-time infrastructure to do some checking on the two systems and ensure they're in sync? That might be a nice warning to emit.

[jwoertink]

Well, I had a few issues going on. Yeah, one was definitely time drift, but the other was the fact that my specs ran too fast. I was inserting a ton of records then sorting them by created_at and getting different results because they were all inserted at the same second. By adding milliseconds, I was able to get the correct sorting.

I haven't had a chance to really put more thought in to this, but it does feel like it was more of a "me" issue than Avram. It's just that I had to monkey patch to get around it. I'm not totally sure on what the proper solution would be.

[robacarp]

I do agree that adding milliseconds is an important feature for Avram.

[robacarp]

This comment from @jwoertink on #732 seems really important to me:

It's actually whatever time zone the machine that the app is running on. The conversion doesn't actually happen until a record is saved to the DB. This means that on queries, there's no conversion. If I insert a record, it's saved as UTC, but then I query it from PST, postgres will compare WHERE some_utc_date <= some_pst_date. I think we would have to manually do the conversion maybe in the Time Criteria?

That's a really critical gotcha! In the rare event where the server and database exist in different timezones this would translate to "mysterious behavior" quickly.

[jwoertink]

Exactly. And to be clear, that's when you do something like created_at.lte(30.days.ago). If you do created_at.lte(Time.utc), then it's technically moot since you've done the conversion ahead of time.

[davidepaolotua]

Just some notes on this, since it popped up in discord.

Postgres timestamptz internally stores the date as UTC. Always. No way around this.

What changes is the "internal" timezone that is used at database or session level.
Now, depending on the usecase, if you need to force dates being sent from the database in a specific timezone, you have two possible options:

• I execute a set time zone 'xxx' at the beginning of the transaction/session IF I want the option of having multiple timezone clients.
• I execute a alter database yyyy set time zone 'yyy' if I foresee that all traffic will come from a single timezone.

To show an example:

davide@Davides-MBP ~ % psql postgres -c "create database test_db"  #create a test database
CREATE DATABASE
davide@Davides-MBP ~ % psql test_db -c "select now();"  #now() returns a timestamptz (my host tz is Eu/Zurich)
              now
-------------------------------
 2021-11-05 16:26:30.848786+01
(1 row)

davide@Davides-MBP ~ % psql test_db -c "set time zone 'UTC'; select now();" # forcing a new timezone for the session
              now
-------------------------------
 2021-11-05 15:26:57.339187+00
(1 row)

davide@Davides-MBP ~ % psql test_db -c "select now();"  # after the session, the default tz stays the same as the original one
              now
-------------------------------
 2021-11-05 16:27:03.545999+01
(1 row)

davide@Davides-MBP ~ % psql test_db -c "alter database test_db set time zone 'Indian/Christmas'" # now, forcing the default tz to something else
ALTER DATABASE
davide@Davides-MBP ~ % psql test_db -c "select now();"    # and the timestamp has a different timezone across all sessions
              now
-------------------------------
 2021-11-05 22:27:58.443664+07
(1 row)

Then, it really depends on your use case, whether it is a "I should support n-different timezones" (so potentially one for each session) or a "all my stuff will be seen in a single timezone" (like everything in CEST). (in which case I usually do a alter database)