Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/polyamorous/activerecord/join_dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@ def construct_tables_for_association!(join_root, association)
tables
end

def populate_joined_tables_from_existing_joins(join_nodes, relation)
@joined_tables ||= {}

join_nodes.each do |join|
next unless join.left.respond_to?(:name)

table_name = join.left.name

# Find reflections that correspond to this joined table
relation.klass.reflect_on_all_associations.each do |reflection|
if reflection.klass.table_name == table_name
# Mark this reflection as already joined with the existing table
@joined_tables[reflection] = [join.left, true]
end
end
end
end

private

def table_aliases_for(parent, node)
Expand Down
4 changes: 4 additions & 0 deletions lib/ransack/adapters/active_record/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ def build_joins(relation)
alias_tracker = relation.alias_tracker(join_list)
join_dependency = Polyamorous::JoinDependency.new(relation.klass, relation.table, association_joins, Arel::Nodes::OuterJoin)
join_dependency.instance_variable_set(:@alias_tracker, alias_tracker)

# Track existing join nodes to prevent duplicate joins
join_dependency.send(:populate_joined_tables_from_existing_joins, join_nodes, relation)

join_nodes.each do |join|
join_dependency.send(:alias_tracker).aliases[join.left.name.downcase] = 1
end
Expand Down
61 changes: 61 additions & 0 deletions spec/ransack/adapters/active_record/redundant_joins_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
require 'spec_helper'

module Ransack
module Adapters
module ActiveRecord
describe 'Redundant Join Prevention' do
describe 'with nested associations' do
context 'when relation already has INNER JOINs' do
it 'should not generate redundant LEFT OUTER JOINs for nested associations' do
# Test case from issue: Comment.joins(article: :author).ransack(article_author_name_eq: 'abc')
# This was generating both INNER JOIN and LEFT OUTER JOIN to authors table
search = Comment.joins(article: :author).ransack(article_author_name_eq: 'test')
sql = search.result.to_sql

# Should contain only one JOIN to authors table
authors_joins = sql.scan(/JOIN "authors"/).length
expect(authors_joins).to eq(1), "Expected 1 join to authors table, but found #{authors_joins} in SQL: #{sql}"

# Should not contain LEFT OUTER JOIN with aliased authors table
expect(sql).not_to match(/LEFT OUTER JOIN "authors" "authors_\w+"/),
"SQL should not contain LEFT OUTER JOIN with aliased authors table: #{sql}"

# Should contain WHERE clause referencing authors.name
expect(sql).to match(/"authors"\."name" = 'test'/),
"SQL should contain WHERE clause for authors.name: #{sql}"
end

it 'should work correctly with simple single-level joins' do
# Test case from issue: Comment.joins(:article).ransack(article_title_eq: 'abc')
# This should continue to work as before (only one JOIN to articles)
search = Comment.joins(:article).ransack(article_title_eq: 'test')
sql = search.result.to_sql

# Should contain only one JOIN to articles table
articles_joins = sql.scan(/JOIN "articles"/).length
expect(articles_joins).to eq(1), "Expected 1 join to articles table, but found #{articles_joins} in SQL: #{sql}"

# Should contain WHERE clause referencing articles.title
expect(sql).to match(/"articles"\."title" = 'test'/),
"SQL should contain WHERE clause for articles.title: #{sql}"
end
end

context 'when relation has no existing joins' do
it 'should generate appropriate LEFT OUTER JOINs for search conditions' do
# When no existing joins, Ransack should create necessary LEFT OUTER JOINs
search = Comment.ransack(article_author_name_eq: 'test')
sql = search.result.to_sql

# Should contain LEFT OUTER JOINs for both articles and authors
expect(sql).to match(/LEFT OUTER JOIN "articles"/),
"SQL should contain LEFT OUTER JOIN for articles: #{sql}"
expect(sql).to match(/LEFT OUTER JOIN "authors"/),
"SQL should contain LEFT OUTER JOIN for authors: #{sql}"
end
end
end
end
end
end
end