Ransack: Incorrect reference to table alias.

Created on 10 Oct 2020  路  7Comments  路  Source: activerecord-hackery/ransack

I know this issue seems related to a couple others, but I think it may involve a slightly novel twist.

I have an indirect association I want to ransack, while also ransacking the intermediate association. It matters greatly what order these query options are provided to .ransack. If they follow the natural ordering (intermediate table first), all is well. If they go in the opposite order, then I wind up with a bad query because the LEFT OUTER JOIN aliases the table as <intermediate_table_name>_<direct_table_name>_join, but the WHERE clause refers to <intermediate_table_name>_<direct_table_name>.

This behavior is novel starting with Rails 6.0.1, and I have a nice little repro case:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  # Issue does NOT occur with Rails 6.0.0
  # Issue DOES occur with Rails 6.0.1, and 6.0.3.4
  gem "activerecord", "6.0.3.4", require: "active_record"
  gem "ransack", "2.3.2"
  gem "sqlite3"

  gem "pry"
end

require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table "users", force: :cascade do |t|
    t.string "email", default: "", null: false
    t.index ["email"], name: "idx_users_on_email", unique: true
  end

  create_table "google_accounts", force: :cascade do |t|
    t.bigint "user_id", null: false
    t.string "email", null: false
    t.index ["email"], name: "idx_g_accounts_on_email"
    t.index ["user_id"], name: "idx_g_accounts_on_user_id"
  end

  create_table "google_calendars", force: :cascade do |t|
    t.bigint "google_account_id", null: false
    t.string "google_id", null: false
    t.index ["google_account_id", "google_id"], name: "idx_g_calendars_on_g_account_id_and_g_id", unique: true
    t.index ["google_account_id"], name: "idx_g_calendars_on_g_account_id"
  end
end

class User < ActiveRecord::Base
  has_many :accounts,
           class_name: "GoogleAccount"
  has_many :contacts,
           class_name: "GoogleContact",
           through:    :accounts,
           source:     :contacts
  has_many :calendars,
           class_name: "GoogleCalendar",
           through:    :accounts,
           source:     :calendars
end

class GoogleAccount < ActiveRecord::Base
  belongs_to :user

  has_many :calendars, class_name: "GoogleCalendar"
end

class GoogleCalendar < ActiveRecord::Base
  belongs_to :account,
             class_name:  "GoogleAccount",
             foreign_key: "google_account_id",
             inverse_of:  :calendars

  has_one :user, through: :account
end


class BugTest < Minitest::Test
  def setup
    user1 = User.create!(email: "[email protected]")
    user2 = User.create!(email: "[email protected]")
    user1_acct1 = GoogleAccount.create!(user: user1, email: "[email protected]")
    user1_acct2 = GoogleAccount.create!(user: user1, email: "[email protected]")
    user2_acct1 = GoogleAccount.create!(user: user2, email: "[email protected]")
    user2_acct2 = GoogleAccount.create!(user: user2, email: "[email protected]")
    user1_acct1_cal1 = GoogleCalendar.create!(account: user1_acct1, google_id: "[email protected]")
    user1_acct1_cal2 = GoogleCalendar.create!(account: user1_acct1, google_id: "[email protected]")
    user1_acct2_cal1 = GoogleCalendar.create!(account: user1_acct2, google_id: "[email protected]")
    user1_acct2_cal2 = GoogleCalendar.create!(account: user1_acct2, google_id: "[email protected]")
    user2_acct1_cal1 = GoogleCalendar.create!(account: user2_acct1, google_id: "[email protected]")
    user2_acct1_cal2 = GoogleCalendar.create!(account: user2_acct1, google_id: "[email protected]")
    user2_acct2_cal1 = GoogleCalendar.create!(account: user2_acct2, google_id: "[email protected]")
    user2_acct2_cal2 = GoogleCalendar.create!(account: user2_acct2, google_id: "[email protected]")
  end

  def teardown
    User.destroy_all
    GoogleAccount.destroy_all
    GoogleCalendar.destroy_all
  end

  def test_good_generated_sql_join
    good_result = GoogleCalendar.
                  ransack({ "account_email_cont" => "account1", "user_email_eq" => "[email protected]" }).
                  result
    # SELECT "google_calendars".*
    # FROM "google_calendars"
    # LEFT OUTER JOIN "google_accounts" ON "google_accounts"."id" = "google_calendars"."google_account_id"
    # LEFT OUTER JOIN "users" ON "users"."id" = "google_accounts"."user_id"
    # LEFT OUTER JOIN "google_accounts" "accounts_google_calendars" ON "accounts_google_calendars"."id" = "google_calendars"."google_account_id"
    # WHERE ("google_accounts"."email" LIKE '%account1%' AND "users"."email" = '[email protected]')

    assert_equal ["[email protected]", "[email protected]"], good_result.map(&:google_id)
  end

  def test_bad_generated_sql_join
    # Reversing the order of the hash fields:
    bad_result = GoogleCalendar.
                 ransack({ "user_email_eq" => "[email protected]", "account_email_cont" => "account1" }).
                 result
    # SELECT "google_calendars".*
    # FROM "google_calendars"
    # LEFT OUTER JOIN "google_accounts" ON "google_accounts"."id" = "google_calendars"."google_account_id"
    # LEFT OUTER JOIN "google_accounts" "accounts_google_calendars_join" ON "accounts_google_calendars_join"."id" = "google_calendars"."google_account_id"
    # LEFT OUTER JOIN "users" ON "users"."id" = "accounts_google_calendars_join"."user_id"
    # WHERE ("users"."email" = '[email protected]' AND "accounts_google_calendars"."email" LIKE '%account1%')

    # Note that the `WHERE` clause refers to `accounts_google_calendars`, but the alias in the join is `accounts_google_calendars_join`!

    assert_equal ["[email protected]", "[email protected]"], bad_result.map(&:google_id)
  end
end

Doing my best to dig through Ransack internals I found Arel objects being created that include TableAliasobjects that have the name accounts_google_calendars, so I started looking for where the _join was added. I believe I found where it's hiding in ActiveRecord, and have a stack trace:

/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency.rb:190:in `table_alias_for'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency.rb:179:in `block in table_aliases_for'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency.rb:176:in `map'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency.rb:176:in `table_aliases_for'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency.rb:158:in `block in construct_tables!'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency/join_part.rb:38:in `block in each_children'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency/join_part.rb:37:in `each'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency/join_part.rb:37:in `each_children'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/activerecord-6.0.3.4/lib/active_record/associations/join_dependency.rb:157:in `construct_tables!'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/adapters/active_record/context.rb:316:in `build_association'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/adapters/active_record/context.rb:280:in `build_or_find_association'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/adapters/active_record/context.rb:205:in `get_parent_and_attribute_name'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/adapters/active_record/ransack/context.rb:40:in `bind_pair_for'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/context.rb:62:in `bind'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:151:in `block in build_attribute'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:150:in `tap'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:150:in `build_attribute'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:83:in `block in attributes='
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:82:in `each'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:82:in `attributes='
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:176:in `block in build'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:174:in `each'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:174:in `build'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/condition.rb:17:in `extract'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/grouping.rb:175:in `write_attribute'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/nodes/grouping.rb:115:in `method_missing'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/search.rb:44:in `block in build'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/search.rb:40:in `each'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/search.rb:40:in `build'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/search.rb:32:in `initialize'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/adapters/active_record/base.rb:18:in `new'
/Users/jfrisby/.rbenv/versions/2.6.6/gemsets/thryve-core/gems/ransack-2.3.2/lib/ransack/adapters/active_record/base.rb:18:in `ransack'
...

I'm not sure where the aliasing is getting out of sync, but somewhere along the way one part of the Arel tree seems to think one thing, and another part thinks another thing.

I wish I could send you a patch, but Ransack and Arel are very complex and I lack enough context to even properly understand what's going on, let alone come up with a patch that won't break other things.

tests wanted
Source
picture MrJoy

Most helpful comment

I changed 6.0.3.4 version to
gem 'rails', github: 'rails/rails', branch: '6-0-stable'
And it works
Currently 6.0.4 hasn't been released

picture istickz on 30 Dec 2020
🎉2

All 7 comments

Thank you @MrJoy could this awesome write-up be converted into a PR with the failing tests?

seanfcarroll picture seanfcarroll on 27 Nov 2020

Based on the git bisect result, this behavior had been introduced by this commit between Rails v6.0.0 and v6.0.1.

https://github.com/rails/rails/commit/dd46d18e2108f2e953e9811c6eabf5b18fe77ab1

Then it has been addressed by this commit, which will be available if Rails 6.0.4 is released.

https://github.com/rails/rails/commit/f9ba52477ca288e7effa5f6794ae3df3f4e982bc

yahonda picture yahonda on 2 Dec 2020
🎉1

That's great to hear. Then maybe we can merge #1176, skipping the failing spec on v6.0.3, similarly to 61913a7f6061242448bd2b694994350624872b68?

deivid-rodriguez picture deivid-rodriguez on 2 Dec 2020

Yes. I think we can merge #1176 and skip this test on v6.0.3.
One concern is it fails against Rails 6.1.0.rc1 then we will need to find how to address these failures on Rails 6.1.

yahonda picture yahonda on 2 Dec 2020
👍2

I see, then we need to work on the fix for sure! I don't care whether we merge the skipped tests first or together with the future fix.

deivid-rodriguez picture deivid-rodriguez on 2 Dec 2020

I changed 6.0.3.4 version to
gem 'rails', github: 'rails/rails', branch: '6-0-stable'
And it works
Currently 6.0.4 hasn't been released

istickz picture istickz on 30 Dec 2020
🎉2

I think this could be a similar issue like #1119. I had the same problems where a table with alias in join wasn't properly referenced in where part.

nikajukic picture nikajukic on 22 Jan 2021
Was this page helpful?
0 / 5 - 0 ratings

Related issues

grillorafael picture grillorafael  路  5Comments
sebaas picture sebaas  路  3Comments
seanfcarroll picture seanfcarroll  路  4Comments
maduhaime picture maduhaime  路  5Comments
seanfcarroll picture seanfcarroll  路  3Comments