Skip to content

Commit

Permalink
Fix wrong table aliases in Rails 6.1 (#1447)
Browse files Browse the repository at this point in the history
This simply mirrors ActiveRecord's "deduplication" of joined
tables, to not use table aliases that are not actually in the
final query's JOIN clause.

Co-authored-by: Sean <[email protected]>
  • Loading branch information
oneiros and scarroll32 authored Oct 23, 2023
1 parent edbe544 commit 169502b
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 4 deletions.
18 changes: 14 additions & 4 deletions lib/polyamorous/activerecord_6.1_ruby_2/join_dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,21 @@ def construct_tables_for_association!(join_root, association)
private

def table_aliases_for(parent, node)
@joined_tables ||= {}
node.reflection.chain.map { |reflection|
alias_tracker.aliased_table_for(reflection.klass.arel_table) do
root = reflection == node.reflection
name = reflection.alias_candidate(parent.table_name)
root ? name : "#{name}_join"
table, terminated = @joined_tables[reflection]
root = reflection == node.reflection

if table && (!root || !terminated)
@joined_tables[reflection] = [table, true] if root
table
else
table = alias_tracker.aliased_table_for(reflection.klass.arel_table) do
name = reflection.alias_candidate(parent.table_name)
root ? name : "#{name}_join"
end
@joined_tables[reflection] ||= [table, root] if join_type == Arel::Nodes::OuterJoin
table
end
}
end
Expand Down
16 changes: 16 additions & 0 deletions spec/ransack/adapters/active_record/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,22 @@ module ActiveRecord
end
end

context 'has_one through associations' do
let(:address) { Address.create!(city: 'Boston') }
let(:org) { Organization.create!(name: 'Testorg', address: address) }
let!(:employee) { Employee.create!(name: 'Ernie', organization: org) }

it 'works when has_one through association is first' do
s = Employee.ransack(address_city_eq: 'Boston', organization_name_eq: 'Testorg')
expect(s.result.to_a).to include(employee)
end

it 'works when has_one through association is last' do
s = Employee.ransack(organization_name_eq: 'Testorg', address_city_eq: 'Boston')
expect(s.result.to_a).to include(employee)
end
end

context 'negative conditions on HABTM associations' do
let(:medieval) { Tag.create!(name: 'Medieval') }
let(:fantasy) { Tag.create!(name: 'Fantasy') }
Expand Down
28 changes: 28 additions & 0 deletions spec/support/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,20 @@ class Account < ApplicationRecord
belongs_to :trade_account, class_name: "Account"
end

class Address < ApplicationRecord
has_one :organization
end

class Organization < ApplicationRecord
belongs_to :address
has_many :employees
end

class Employee < ApplicationRecord
belongs_to :organization
has_one :address, through: :organization
end

module Schema
def self.create
ActiveRecord::Migration.verbose = false
Expand Down Expand Up @@ -300,6 +314,20 @@ def self.create
t.belongs_to :agent_account
t.belongs_to :trade_account
end

create_table :addresses, force: true do |t|
t.string :city
end

create_table :organizations, force: true do |t|
t.string :name
t.integer :address_id
end

create_table :employees, force: true do |t|
t.string :name
t.integer :organization_id
end
end

10.times do
Expand Down

0 comments on commit 169502b

Please sign in to comment.