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.

Sign up for GitHub

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

Jump to bottom

connection.select_all binds regression #27923

Closed
RustamF1 opened this issue Feb 6, 2017 · 11 comments
Closed

connection.select_all binds regression #27923

RustamF1 opened this issue Feb 6, 2017 · 11 comments
Labels
activerecord attached PR regression
Milestone
5.0.x

Comments

@RustamF1
Copy link

RustamF1 commented Feb 6, 2017 •
edited

Steps to reproduce

rails new test
cd test
rails g model post body:string
rake db:migrate
rails console
Post.connection.select_all('SELECT id FROM posts WHERE id > $1', 'SQL', [[nil, 0]])

Expected behavior

This normally work in Rails 4.2.7.1

Actual behavior

NoMethodError: undefined method `value_for_database' for [nil, 0]:Array
        from ~/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.1/lib/active_record/connection_adapters/sqlite3_adapter.rb:191:in `block in exec_query'
        from ~/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.1/lib/active_record/connection_adapters/sqlite3_adapter.rb:191:in `map'
        from ~/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.1/lib/active_record/connection_adapters/sqlite3_adapter.rb:191:in `exec_query'
        from ~/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:373:in `select'
        from ~/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:41:in `select_all'
        from ~/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.1/lib/active_record/connection_adapters/abstract/query_cache.rb:95:in `select_all'
        from (irb):1
        from ~/.rvm/gems/ruby-2.3.1/gems/railties-5.0.1/lib/rails/commands/console.rb:65:in `start'
        from ~/.rvm/gems/ruby-2.3.1/gems/railties-5.0.1/lib/rails/commands/console_helper.rb:9:in `start'
        from ~/.rvm/gems/ruby-2.3.1/gems/railties-5.0.1/lib/rails/commands/commands_tasks.rb:78:in `console'
        from ~/.rvm/gems/ruby-2.3.1/gems/railties-5.0.1/lib/rails/commands/commands_tasks.rb:49:in `run_command!'
        from ~/.rvm/gems/ruby-2.3.1/gems/railties-5.0.1/lib/rails/commands.rb:18:in `<top (required)>'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:293:in `require'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:293:in `block in require'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:259:in `load_dependency'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:293:in `require'
... 1 levels...
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:287:in `load'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:287:in `block in load'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:259:in `load_dependency'
        from ~/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.1/lib/active_support/dependencies.rb:287:in `load'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/commands/rails.rb:6:in `call'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/command_wrapper.rb:38:in `call'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application.rb:191:in `block in serve'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application.rb:161:in `fork'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application.rb:161:in `serve'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application.rb:131:in `block in run'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application.rb:125:in `loop'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application.rb:125:in `run'
        from ~/.rvm/gems/ruby-2.3.1/gems/spring-2.0.1/lib/spring/application/boot.rb:19:in `<top (required)>'
        from ~/.rvm/rubies/ruby-2.3.1/lib/ruby/site_ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from ~/.rvm/rubies/ruby-2.3.1/lib/ruby/site_ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from -e:1:in `<main>'

System configuration

Rails version:
5.0.1
Ruby version:
ruby 2.3.1
@al2o3cr
Copy link
Contributor

al2o3cr commented Feb 6, 2017

What did this do prior to 5.0.1?

@RustamF1
Copy link
Author

RustamF1 commented Feb 6, 2017 •
edited 

$ rails console
Loading development environment (Rails 4.2.7.1)

 Post.connection.select_all("select id from posts where id > $1", "SQL", [[nil, 0]])
  SQL (0.2ms)  select id from posts where id > $1  [[nil, 0]]
 => #<ActiveRecord::Result:0x00000003eb16a0 @columns=["id"], @rows=[[1]], @hash_rows=nil, @column_types={}>

 p = Post.create(body: 'first')

 Post.connection.select_all("select id from posts where id > $1", "SQL", [[nil, 0]]).each{ |p| puts p['id'] }
  SQL (0.9ms)  select id from posts where id > $1  [[nil, 0]]
1
 => [{"id"=>1}]

@RustamF1
Copy link
Author

RustamF1 commented Feb 7, 2017 •
edited

I found new syntax for 'exec_query' binds in Rails 5.0
activerecord/test/cases/bind_parameter_test.rb#L42

universal solution for Rails 5.0.1 and older versions

if Rails.version > "5.0"
  binds = [ActiveRecord::Relation::QueryAttribute.new("blabla", 0, ActiveRecord::Type::Value.new)]
else
  binds = [[nil, 0]]
end

Post.connection.select_all('SELECT id FROM posts WHERE id > $1', 'SQL', binds).each{ |p| puts p['id'] }

 > SQL (0.3ms)  SELECT id FROM posts WHERE id > $1  [["blabla", 1]]

or create function

  def add_bind(binds, value, name = nil)
    unless Rails.version > "5.0"
      return binds << [nil, value]
    end
      
    type = ActiveRecord::Type::Value.new
    name ||= "p#{binds.length + 1}"
    
    binds << ActiveRecord::Relation::QueryAttribute.new(name, value, type)
  end

and use it

binds = []
add_bind(binds, 0, 'blabla')

Post.connection.select_all('SELECT id FROM posts WHERE id > $1', 'SQL', binds)

@rafaelfranca
Copy link
Member

rafaelfranca commented Feb 7, 2017 •
edited

select_all is low level API, why do you need to use this API instead of the where("'SELECT id FROM posts WHERE id > ?", [0]) API?

@RustamF1
Copy link
Author

RustamF1 commented Feb 7, 2017

This sample (simple) for reproduce problem for others.
In real project, I used custom difficult SQL query for get an array of hashes

@matthewd
Copy link
Member

matthewd commented Feb 7, 2017

The connection/adapter API is lower level than the model API, but it's still a published API.

@rafaelfranca
Copy link
Member

Got it. It was a breaking change in 5.0 but I'm more inclined to not make the behavior backwards compatible. But it should be worth to document what is the current API to use this method. Could you open a PR with the documentation?

@rafaelfranca
Copy link
Member

As a second part we can restore the behavior where binds are arrays in all the public methods that accept binds

@rafaelfranca rafaelfranca added this to the 5.0.x milestone Feb 7, 2017
@RustamF1
Copy link
Author

RustamF1 commented Feb 7, 2017

I think you just need to specify this in the documentation.
Or in 5_0_release_notes

@rafaelfranca
Copy link
Member

@matthewd had a good point that we still can make it backwards compatible. So I marked it as a regression.

@kamipo kamipo mentioned this issue Feb 7, 2017
Merged
kamipo added a commit to kamipo/rails that referenced this issue Feb 12, 2017
@kamipo
Fix select_all with legacy binds
082c75f 
Fixes rails#27923.
@kamipo
Copy link
Member

kamipo commented Mar 5, 2017

#27939 fixes the regression.

kamipo added a commit to kamipo/rails that referenced this issue Mar 20, 2017
@kamipo
Fix select_all with legacy binds
2f8cef9 
Fixes rails#27923.
@JoshCheek JoshCheek mentioned this issue Jan 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
activerecord attached PR regression
Projects
None yet
Milestone
5.0.x
Development

No branches or pull requests
6 participants
@matthewd @al2o3cr @kamipo @rafaelfranca @maclover7 @RustamF1