crystal-lang / crystal-mysql

MySQL connector for Crystal
MIT License
107 stars 36 forks source link

Support for MariaDB? #99

Open luislavena opened 3 years ago

luislavena commented 3 years ago

Hello folks,

Recently had the need to migrate from MySQL 5.7 to MariaDB (10.3 at first, 10.5 at some point), and wanted to confirm crystal-mysql adapter will work against it.

When running specs, encountered only 2 failures:

  1) as a db select 1 as bigint as literal

       MySql::ResultSet#read returned a Int32. A Int64 was expected. (Exception)
         from lib/db/src/db/result_set.cr:80:7 in 'read'
         from /spec.cr:5:3 in 'assert_single_read'
         from lib/db/src/spec.cr:164:13 in '->'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/primitives.cr:255:3 in '->'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/primitives.cr:255:3 in 'internal_run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/example.cr:33:16 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:330:7 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:147:7 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/dsl.cr:274:7 in '->'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/primitives.cr:255:3 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/crystal/main.cr:45:14 in 'main'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/crystal/main.cr:119:3 in 'main'
         from __libc_start_main
         from _start
         from ???

  2) as a db select -1 as bigint as literal

       MySql::ResultSet#read returned a Int32. A Int64 was expected. (Exception)
         from lib/db/src/db/result_set.cr:80:7 in 'read'
         from /spec.cr:5:3 in 'assert_single_read'
         from lib/db/src/spec.cr:164:13 in '->'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/primitives.cr:255:3 in '->'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/primitives.cr:255:3 in 'internal_run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/example.cr:33:16 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:330:7 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/context.cr:147:7 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/spec/dsl.cr:274:7 in '->'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/primitives.cr:255:3 in 'run'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/crystal/main.cr:45:14 in 'main'
         from /home/luis/.asdf/installs/crystal/1.0.0/share/crystal/src/crystal/main.cr:119:3 in 'main'
         from __libc_start_main
         from _start
         from ???

Before I commit some time to this, I would like to know if MariaDB should be considered supported (given the current MySQL 8 situation about authentication plugins) and if so, then I could contribute the required fixes.

Thank you in advance. ❤️ ❤️ ❤️

bcardiff commented 3 years ago

I'm fine accepting maria-db issues. It would like it to be an alternative for users.

Those specs that are failing are because it seems mariadb and mysql do not agree on the type on a select 1.

The following works with maria-db (but will not with mysql)

  sample_value 1, "int", "1" # , type_safe_value: false
  sample_value 1_i64, "bigint", "1", type_safe_value: false
  # ...
  sample_value -1, "int", "-1" # , type_safe_value: false
  sample_value -1_i64, "bigint", "-1", type_safe_value: false

With type_safe_value: true (default) the select_scalar_syntax will be used with the expression directly, in this case select 1 and select -1. And the specs will validate the result set should read the first column of the first row will have the same value and type as the first argument. So mysql returns something that translates to Int64 while maria-db something that translates to Int32.

I could not find any documentation regarding this differences.

So the actionable thing is probably to:

  1. Add mariadb to the CI
  2. Make specs pass on both
bcardiff commented 3 years ago

And to be clear that means crystal-mysql would not be able to guarantee the type of a scalar value. Which is not great. But I think this can be handled by crystal-db directly if we allow some type conversions (ie: rs.read returned an Int32, but the user is calling rs.read(Int64), I imagine this to allow runtime type mismatch since that is already the case if db types are different from what user wrote in the code).

luislavena commented 3 years ago

Hello @bcardiff, thanks for your response and your patience, missed this in my inbox.

Thanks for providing the details, I was able to find the reference about that change in MDEV-16347, indicating that the change was intentional from MDEV-12619.

Perhaps the spec/test can be adjusted to evaluate first the data type of CREATE TABLE t1 AS SELECT 1 AS c1 and from there determine the type?

Cheers.