Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for MariaDB? #99

Open
luislavena opened this issue May 3, 2021 · 3 comments
Open

Support for MariaDB? #99

luislavena opened this issue May 3, 2021 · 3 comments

Comments

@luislavena
Copy link

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.
❤️ ❤️ ❤️

@luislavena luislavena changed the title Support for MariaDB Support for MariaDB? May 3, 2021
@bcardiff
Copy link
Member

bcardiff commented May 4, 2021

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
Copy link
Member

bcardiff commented May 4, 2021

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants