elixir-sqlite/ecto_sqlite3

:utc_datetime handling

Closed this issue · 4 comments

I'm currently in the process of migrating an sqlite_ecto2 project to this adapter and I noticed that this one does seem to store utc_datetime as iso strings with tz information (with trailing Z), while sqlite_ecto2 doesn't store an offset.

The comment here seems to suggest this is not yet set in stone:

# TODO: Should we be preserving the timezone? SQLite3 stores everything
# shifted to UTC. sqlite_ecto2 used a custom field type "TEXT_DATETIME"
# to preserve the original string inserted. But I don't know if that
# is desirable or not.
#
# @warmwaffles 2021-02-28

I'd argue that the adapter doesn't want to push timezone knowledge into the db, just like the other ecto adapters do. Neither postgres nor mysql natively support a datetime column, which does retain timezone information. Even though timestamptz for postgres does convert between session timezone and the column value automatically, it also just stores UTC and not the input timezone – and ecto defaults to timestamp columns on postgres anyways. Additionally the :utc_datetime ecto type already makes sure the datetime is using UTC as timezone at runtime, so other timezones are not allowed for such columns in the first place.

Would you be open to adjusting the handling for :utc_datetime?

A PR is always welcome. I think @kevinlang may have some thoughts on this as well. The current serialization of timestamps and datetimes just go through the elixir datetime module and formats it to iso8601

https://hexdocs.pm/ecto/Ecto.Schema.html#module-the-datetime-types

All of those types are represented by the same timestamp/datetime in the underlying data storage, the difference are in their precision and how the data is loaded into Elixir.

Makes sense per the above. PR welcome! :)

I've created a PR to change the handling as requested.

The other difference I noticed are that the column types used between sqlite_ecto2 and ecto_sqlite3 are different (NAIVE_DATETIME/UTC_DATETIME vs TEXT_DATETIME). By now I've not found this to cause issues, but I guess those sql column types are used by exqlite/ecto_sqlite3 in some form. Would I need to convert/migrate existing tables/data?

Thanks for the PRs @LostKobrakai ! 🥳