Check all children of Arel::Nodes::And to extract correlated key#1572
Conversation
riseshia
force-pushed
the
fix-extract-correlated-key-with-and-node-has-multiple-children
branch
from
365c324 to
e471827
Compare
|
fix tests with mysql, which won't be failed e471827 |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where correlated subqueries fail to extract the proper key when Arel::Nodes::And nodes have more than two children. The fix changes from accessing only left and right children to iterating through all children in the children array.
- Updates the
extract_correlated_keymethod to handleArel::Nodes::Andnodes with multiple children - Adds test coverage for polymorphic associations with default scopes
- Adds a
RecentNotemodel to support the new test case
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/ransack/adapters/active_record/context.rb | Updates extract_correlated_key to iterate through all children of Arel::Nodes::And nodes |
| spec/support/schema.rb | Adds RecentNote model and recent_notes association for testing |
| spec/ransack/adapters/active_record/context_spec.rb | Adds test case for polymorphic associations with default scopes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| when Arel::Nodes::And | ||
| extract_correlated_key(join_root.left) || extract_correlated_key(join_root.right) | ||
| # And may have multiple children, so we need to check all, not via left/right | ||
| join_root.children.each do |child| |
There was a problem hiding this comment.
@riseshia what is the potential for performance issues here?
There was a problem hiding this comment.
no performance issue here, it just has possibility that join_root have more than 2 children.
…-multiple-children
…-multiple-children
riseshia
deleted the
fix-extract-correlated-key-with-and-node-has-multiple-children
branch
🔗 #1571
This is proposed patch for #1571. Check issue for the detail of this.