elixir-sqlite/ecto_sqlite3

Handling of UUIDs

Closed this issue · 5 comments

While making sure we don't run into compatibility issues migrating from the sqlite_ecto2 adapter to this one I hit another difference: UUIDs.

Currently this adapter seems to store uuids as strings and the old adapter did opt for the smaller option of raw binary data. I found this one in the testsuite:

# TODO: We need to determine what format to store the UUID. Is it Text or binary 16?
# Are we going for readability or for compactness?

I'm wondering if you already decided for one way or the other? It would be really great if this adapter could be switched to using the binary-16 storage. In the embedded world storing data in a more than twice as big format doesn't sound appealing.

I've added the appropriate loaders/2 and dumpers/2 callback locally to use Ecto.UUID and it seems nothing in the testsuite broke and it made my compatibility check between the adapters work.

My only concern with using the binary is that it is much harder to work with when using sqlite3 cli. AFAIK there are no built in utilities to query a UUID by its text representation within the CLI, unlike Postgres and MySQL. It is only possible by using the UUID extension, but most 99.9% of users will be using the distro-provided sqlite3 binary which will not have this.

I think it is reasonable to make it configurable - Postgres does this with the :map type.

    defp ecto_to_db(:map),                 do: Application.fetch_env!(:ecto_sql, :postgres_map_type)
    defp ecto_to_db({:map, _}),            do: Application.fetch_env!(:ecto_sql, :postgres_map_type)

We can do this by doing:

  def column_type(:uuid, _opts), do: Application.get_env(:ecto_sql, :uuid_type, "TEXT")
  def column_type(:binary_id, _opts), do: Application.get_env(:ecto_sql, :binary_id_type, "TEXT")

What do you think, @LostKobrakai ?

I'm happy to leave it configurable for as long as we can somehow use the raw binary representation, though the column type is only part of the picture, the encoding/decoding is the bigger one :)

Yeah, I think the loader/dumper can just look at that same config to determine which way to process it.

Yea I'd be okay with a configurable option with how to handle that.

We could also do the same thing for TEXT_DATETIME to be just DATETIME if people wanted.

It is only possible by using the UUID extension, but most 99.9% of users will be using the distro-provided sqlite3 binary which will not have this.

@kevinlang looking at the code in that extension, it looks like it is fallback safe and stores the values as bytes. But again, it won't be human friendly for those using a client without the extension. It would also make it incredibly difficult to insert a new record through the CLI for maintenance and what not.