activerecord-hackery / squeel

Active Record, improved. Live again :)
http://erniemiller.org/2013/11/17/anyone-interested-in-activerecord-hackery/
MIT License
2.4k stars 214 forks source link

Rails 4.2.4 - Mixing attribute values #400

Open brunobrgs opened 9 years ago

brunobrgs commented 9 years ago

First of all, the problem starts with a duplication on sql condition on rails 4.2, when i call the same scope twice. Following this issue https://github.com/rails/rails/issues/21626

When i updated my rails to 4.2.4, the sql generated is mixing the attribute values, like script below shows:

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  gem 'rails', '4.2.4', require: 'active_record'
  gem 'sqlite3'
  gem 'arel'
  gem 'squeel', '1.2.3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true  do |t|
  end

  create_table :comments, force: true  do |t|
    t.integer :post_id
    t.string :classification
    t.string :nature
    t.boolean :active
  end
end

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post

  scope :active, -> value = true { where(active: value) }
  scope :by_classification, -> value { where(classification: value) }
  scope :by_nature, -> value { where(nature: value) }
  scope :mix, -> { active.by_classification('A').by_nature('B') }
end

class BugTest < Minitest::Test
  def test_association_stuff
    post = Post.create!
    post.comments << Comment.create!

    #1 Call
    sql1 = "SELECT \"comments\".* FROM \"comments\" WHERE \"comments\".\"active\" = 't' AND \"comments\".\"classification\" = 'A' AND \"comments\".\"nature\" = 'B'"
    assert_equal sql1, Comment.mix.to_sql

    #2 Calls
    sql2 = "SELECT \"comments\".* FROM \"comments\" WHERE \"comments\".\"active\" = 't' AND \"comments\".\"active\" = 'A' AND \"comments\".\"classification\" = 'B' AND \"comments\".\"classification\" = 't' AND \"comments\".\"nature\" = 'A' AND \"comments\".\"nature\" = 'B'"
    assert_equal sql2, Comment.mix.mix.to_sql
  end
end
fschwahn commented 8 years ago

I just spent half an hour creating almost the exact same repro-case - I should have checked here first, but I actually thought it was an issue in activerecord itself, as I was using no squeel behavior, I was just including the gem.

I really like squeel, but I have run into problems with it time and time again. This is not at all the fault of squeel, as the reasons are mainly that activerecord internals can change even in minor versions enough to break squeel (in this case going from 4.2.2 to 4.2.3). Which is also not the fault of the rails developers, these are internals after all which can change at any time. Just starting to realize that heavily patching rails internals is a game you'll eventually lose.

In newer projects I've started to just use arel directly if I need anything rails doesn't provide - the syntax is not even close to squeel in terms of verbosity and readability, but it seems to survive even major rails updates without problems.

konk303 commented 8 years ago

I was trying to upgrade rails from 4.1 to 4.2 and came to this problem. I tried versions from 4.2.5 down to 4.2.0 and it's all there, so I assume this is introduced with rails 4.2.0, and happens ever since.

# I'm using mysql 5.6

# squeel 1.2.3 with rails 4.1.14
[1] pry(main)> User.where(id: 1).where(email: 'foo@example.com').where(id: 1).to_sql
=> "SELECT `users`.* FROM `users`  WHERE `users`.`id` = 1 AND `users`.`email` = 'foo@example.com'"

# squeel 1.2.3 with rails 4.2.(0-5)
[1] pry(main)> User.where(id: 1).where(email: 'foo@example.com').where(id: 1).to_sql
=> "SELECT `users`.* FROM `users` WHERE `users`.`id` = 1 AND `users`.`id` = 'foo@example.com' AND `users`.`email` = 1"

https://github.com/rails/rails/issues/21626 is surely happening, and that spec change might have broken squeel.

This one's preventing me from upgrading rails to 4.2 ...

Skulli commented 8 years ago

Having the same problem on several queries where the attributes are getting mixed.

Skulli commented 7 years ago

@farrspace thanks for the fix, thats working for me :+1: