trilogy-libraries / activerecord-trilogy-adapter

Active Record adapter for the Trilogy database client for Rails v6.0 - v7.0.
https://github.com/trilogy-libraries/trilogy
MIT License
167 stars 16 forks source link

Prepared statements issue with Rails main and MySQL 5.7.39 #5

Closed zavan closed 1 year ago

zavan commented 1 year ago

When I run a query in the Rails console with trilogy:

irb(main):001:0> User.last
  User Load (1.2ms)  SELECT `users`.* FROM `users` ORDER BY `users`.`id` DESC LIMIT ?
/usr/local/bundle/gems/activerecord-trilogy-adapter-2.0.0/lib/active_record/connection_adapters/trilogy_adapter.rb:153:in `query': 
Trilogy::DatabaseError: trilogy_query_recv: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1 (ActiveRecord::StatementInvalid)
irb(main):004:0> User.where(email: 'example@example.com')
  User Load (0.8ms)  SELECT `users`.* FROM `users` WHERE `users`.`email` = ? /* loading for pp */ LIMIT ?
(Object doesn't support #inspect)                                      
=>

irb(main):005:0> User.where(email: 'example@example.com').to_a
  User Load (0.8ms)  SELECT `users`.* FROM `users` WHERE `users`.`email` = ?
/usr/local/bundle/gems/activerecord-trilogy-adapter-2.0.0/lib/active_record/connection_adapters/trilogy_adapter.rb:153:in `query': Trilogy::DatabaseError: trilogy_query_recv: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1 (ActiveRecord::StatementInvalid)
/usr/local/bundle/gems/activerecord-trilogy-adapter-2.0.0/lib/active_record/connection_adapters/trilogy_adapter.rb:153:in `query': trilogy_query_recv: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1 (Trilogy::DatabaseError)

Looks like it's missing the values for the prepared statements?

When I run it with mysql2:

irb(main):001:0> User.last
DEPRECATION WARNING: The default value of `prepared_statements` for the mysql2 adapter will be changed from +false+ to +true+ in Rails 7.2. (called from run at /usr/local/bundle/gems/thor-1.2.1/lib/thor/command.rb:27)
  User Load (10.2ms)  SELECT `users`.* FROM `users` ORDER BY `users`.`id` DESC LIMIT ?  [["LIMIT", 1]]
=> nil
irb(main):002:0> User.where(email: 'example@example.com')
  User Load (3.2ms)  SELECT `users`.* FROM `users` WHERE `users`.`email` = ? /* loading for pp */ LIMIT ?  [["email", "example@example.com"], ["LIMIT", 11]]                                             
=> [] 

I'm using Rails from the master branch and Ruby 3.1.2.

Thanks for this open-sourcing this BTW, it really looks promising!

ilianah commented 1 year ago

Hey @zavan I think this might have been an issue with Rails that should be solved by https://github.com/rails/rails/pull/45945. Would you mind trying again? Thanks!

zavan commented 1 year ago

Hi @ilianah, thanks but it's still happening with the default db config:

gem 'activerecord-trilogy-adapter'
gem 'rails', github: 'rails/rails'
Using rails 7.1.0.alpha from https://github.com/rails/rails.git (at main@3d01986)
default: &default
  # adapter: mysql2
  adapter: trilogy
  encoding: utf8mb4
  pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
  username: <%= ENV.fetch("DATABASE_USERNAME") %>
  password: <%= ENV.fetch("DATABASE_PASSWORD") %>
  host: <%= ENV.fetch("DATABASE_HOST") %>
  port: <%= ENV.fetch("DATABASE_PORT") %>
  # prepared_statements: false

development:
  <<: *default
  database: api_development
Loading development environment (Rails 7.1.0.alpha)
irb(main):001:0> User.last
  User Load (0.4ms)  SELECT `users`.* FROM `users` ORDER BY `users`.`id` DESC LIMIT ?
/usr/local/bundle/gems/activerecord-trilogy-adapter-2.0.0/lib/active_record/connection_adapters/trilogy_adapter.rb:153:in `query': Trilogy::DatabaseError: trilogy_query_recv: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1 (ActiveRecord::StatementInvalid)                                                                           
/usr/local/bundle/gems/activerecord-trilogy-adapter-2.0.0/lib/active_record/connection_adapters/trilogy_adapter.rb:153:in `query': trilogy_query_recv: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1 (Trilogy::DatabaseError)

I also tried changing config.load_defaults to 7.1, 7.0 and 6.1, but it made no difference.

However, when I explicitly disable it with prepared_statements: false in the db config it works.

It seems from your PR that it defaults to true in the abstract adapter and false in mysql adapter, but trilogy inherits from the abstract one, so it's defaulting to true. So either the trilogy adapter should override default_prepared_statements to false, or the abstract adapter should default to false.

eileencodes commented 1 year ago

This adapter can't support prepared statements until https://github.com/github/trilogy/pull/3 is merged. But the trilogy adapter should just be updated to set prepared_statements to false until then.

zavan commented 1 year ago

@eileencodes Got it. But shouldn't the adapter default to prepared_statements: false while that's not released (and handle the version differences once it's released) so it works without that extra configuration?

At a minimum this requirement should be added to the Setup section of the README.

Edit: just saw your edit. :)

lorint commented 1 year ago

Have been busy adding compatibility for older Rails versions (5.2 and up) in PR #20. Along the way I had originally tackled this issue in the same way where you had success, @zavan -- the same as what @ilianah had also described -- by using prepared_statements: false. Now after further exploration have found a better way -- to create a patch so that when Trilogy runs under Rails < 7.1 then it honours default_prepared_statements. Here are the relevant lines in that PR:

https://github.com/github/activerecord-trilogy-adapter/blob/3769759ffe88890b293897dae4f5e270f1a9fd00/lib/trilogy_adapter/backwards_ar_compatibility.rb#L51-L54

Would be curious to know if this update works for you, especially with AR 6.x or 7.0.

Thanks :) 🐬