-
-
Notifications
You must be signed in to change notification settings - Fork 827
Description
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
endDoing 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.