Allow to unscope where conditions by arel_table with symbol

[deeeki]

This commit fixes the following case.

User.where(User.arel_table[:created_at].lteq(1.year.ago)).unscope(where :created_at)

reproduction on rails console

[1] pry(main)> User.where(User.arel_table['created_at'].lteq(1.year.ago)).unscope(where: :created_at).to_sql
=> "SELECT `users`.* FROM `users`"
[2] pry(main)> User.where(User.arel_table[:created_at].lteq(1.year.ago)).unscope(where: :created_at).to_sql
=> "SELECT `users`.* FROM `users`  WHERE (`users`.`created_at` <= '2013-12-02 11:43:25')"

I guess it works before this commit. 492bad7

[rafaelfranca]

Could you add a test case not using arel_table? arel_table is private API of Rails and should not be used in application. Your fix is correct if we can trigger the bug without using arel_table but if we can't I prefer to not merge it.

[deeeki]

Hmm, I can't. Using where with Hash like User.where(created_at: 10.years.ago..1.year.ago) turns to Arel::Node with String column name.

But there are some test cases using arel_table with Symbol.

$ git grep 'arel_table\[:'
activerecord/lib/active_record/core.rb:      #     scope :published_and_commented, -> { published.and(self.arel_table[:comments_count].gt(0)) }
activerecord/test/cases/base_test.rb:    count = Weird.group(Weird.arel_table[:from]).count
activerecord/test/cases/calculations_test.rb:    c = Account.group(Account.arel_table[:firm_id]).sum(:credit_limit)
activerecord/test/cases/relation/merging_test.rb:    devs = Developer.where(Developer.arel_table[:salary].eq(80000)).merge(
activerecord/test/cases/relation/merging_test.rb:      Developer.where(Developer.arel_table[:salary].eq(9000))
activerecord/test/cases/relation/merging_test.rb:    salary_attr = Developer.arel_table[:salary]
activerecord/test/cases/relation_test.rb:      relation.where! Comment.arel_table[:id].eq(10)
activerecord/test/cases/relations_test.rb:    topics = Topic.order(Topic.arel_table[:id].asc)
activerecord/test/cases/relations_test.rb:    topics = Topic.order(Topic.arel_table[:id].asc)
activerecord/test/cases/relations_test.rb:    assert_equal Post.order(Post.arel_table[:title]).to_a, Post.order("title").to_a
activerecord/test/cases/scoping/relation_scoping_test.rb:    assert_equal Developer.order("id DESC").to_a.reverse, Developer.order(Developer.arel_table[:id].desc).reverse_order
activerecord/test/cases/scoping/relation_scoping_test.rb:    assert_equal Developer.order("id DESC").order("name DESC").to_a.reverse, Developer.order(Developer.arel_table[:id].desc).order(Developer.arel_table[:name].desc).reverse_order
activerecord/test/cases/scoping/relation_scoping_test.rb:    assert_equal Developer.order("id DESC").order("name DESC").to_a.reverse, Developer.order("id DESC").order(Developer.arel_table[:name].desc).reverse_order
activerecord/test/models/author.rb:           -> { where(title: 'Welcome to the weblog').where(Post.arel_table[:comments_count].gt(0)) },

It causes misunderstanding.

IMO, even though arel_table is private API, building conditions with arel_table is preferable because it is more flexible than String. User.where('created_at <= ?', 1.year.ago) can't unscope specifying created_at column.

[rafaelfranca]

Inside the framework it is allowed to use the private API. The point is, right now application can use it but we don't have any guarantee these methods will be not removed, the behaviour changed without notice, or even renamed.

I know there are a lot of things that can only be done using arel_table but instead of allowing people to use it, I prefer to define a public API that can be as powerful than the private API.

About your specific case, lets add a test using arel_table for now.

[deeeki]

Thanks, I understand your point.
I'll add a test to activerecord/test/cases/scoping/default_scoping_test.rb (in test_unscope_with_where_attributes)
Please let me know if any other location is suitable.

[rafaelfranca]

Seems a good place

[deeeki]

Thanks. Added tests and rebased.