Draft
Conversation
Co-authored-by: scarroll32 <11340230+scarroll32@users.noreply.github.com>
Co-authored-by: scarroll32 <11340230+scarroll32@users.noreply.github.com>
Copilot stopped work on behalf of
scarroll32 due to an error
September 24, 2025 17:23
scarroll32
requested changes
Sep 25, 2025
Member
scarroll32
left a comment
There was a problem hiding this comment.
@copilot the CI is failing
…cates Co-authored-by: scarroll32 <11340230+scarroll32@users.noreply.github.com>
Contributor
Author
Fixed the CI issue. The problem was with my original implementation - I was using a complex approach with method visibility issues. I've simplified the fix to a single line change that addresses the root cause: the The fix in commit 0e25523 removes the join type restriction so all joins are tracked, preventing duplicates. This is much cleaner and less likely to cause test failures. |
Copilot
AI
changed the title
[WIP] Ransack generates redundant left join statements
Fix Ransack redundant left join statements
Sep 25, 2025
| join_dependency.instance_variable_set(:@alias_tracker, alias_tracker) | ||
|
|
||
| # Track existing join nodes to prevent duplicate joins | ||
| join_dependency.populate_joined_tables_from_existing_joins(join_nodes, relation) |
Contributor
There was a problem hiding this comment.
NoMethodError: undefined method 'populate_joined_tables_from_existing_joins' for an instance of ActiveRecord::Associations::JoinDependency
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix for Redundant Left Join Issue
Problem:
Ransack was generating redundant LEFT OUTER JOIN statements when INNER JOINs to the same table already existed in the relation.
Root cause:
The
@joined_tableshash inJoinDependencywas only tracking OuterJoin types, not InnerJoins, causing Ransack to create new LEFT OUTER JOINs to tables that were already joined with INNER JOINs.Solution:
table_aliases_formethod to track ALL join types in@joined_tables, not just OuterJoinsChanges:
lib/polyamorous/activerecord/join_dependency.rb: Removed condition that only tracked OuterJoins in@joined_tablesspec/ransack/adapters/active_record/redundant_joins_spec.rb: Added comprehensive tests to prevent regressionBefore Fix:
After Fix:
The fix ensures that when a relation already has INNER JOINs to certain tables, Ransack will reuse those tables instead of creating redundant LEFT OUTER JOINs.
Fixes #1433
Fixes #1433
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.