-
-
Notifications
You must be signed in to change notification settings - Fork 829
Fix nested conditions, polymorphic associations handling #1561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
def not_nested_condition(attribute, parent_table) | ||
parent_table.class != Arel::Nodes::TableAlias && attribute.name.starts_with?(parent_table.name) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my scenario, this method returns true:
[30] pry(#<Ransack::Nodes::Condition>)> parent_table.class
=> Arel::Table
[31] pry(#<Ransack::Nodes::Condition>)> parent_table.name
=> "tag_values"
[32] pry(#<Ransack::Nodes::Condition>)> attribute.name
=> "tag_values_uid"
And then, when build_correlated_subquery
is called, it crashes with:
undefined method 'eq' for an instance of Array
For context, here's the attribute:
Attribute <tag_values_uid>
And the parent_table:
#<Arel::Table:0x0000000158b97680
@klass=RequestTag(id: integer, uid: string, ...),
@name="tag_values",
@table_alias=nil,
@type_caster=#<ActiveRecord::TypeCaster::Map:0x000000014d011960 @klass=RequestTag(...)>>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, any chance for additional details on data structure and reproduction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the build_correlated_subquery
method, correlated_key
is an array of Arel::Nodes::Equality
elements:
[1] pry(#<Ransack::Adapters::ActiveRecord::Context>)> correlated_key.class
=> Array
[2] pry(#<Ransack::Adapters::ActiveRecord::Context>)> correlated_key.map(&:class)
=> [Arel::Nodes::Equality, Arel::Nodes::Equality]
[3] pry(#<Ransack::Adapters::ActiveRecord::Context>)> correlated_key.map(&:to_sql)
=> ["\"user_relationships\".\"kind\" = $1", "\"user_relationships\".\"source_uid\" = \"tasks\".\"uid\""]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take a look into that at the end of my working day today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way you can help me is by providing parts from schema.rb.related to tables included into query and ransack query itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's another example, with the data structure:
We are trying to filter the tasks
by users_uid_not_in: ["uid_example"]
.
class Task < ApplicationRecord
has_many :follows, primary_key: :uid, inverse_of: :followed, foreign_key: :followed_uid
has_many :users, through: :follows, source: :follower, source_type: User.name
end
# join table
class Follow < ApplicationRecord
belongs_to :follower, polymorphic: true, foreign_key: :follower_uid, primary_key: :uid
belongs_to :followed, polymorphic: true, foreign_key: :followed_uid, primary_key: :uid
end
class User < ApplicationRecord
has_many :follows, primary_key: :uid, inverse_of: :follower, foreign_key: :follower_uid
has_many :tasks, through: :follows, source: :followed, source_type: Task.name
end
# schema.rb
create_table "follows", force: :cascade do |t|
t.string "followed_uid", null: false
t.string "followed_type", null: false
t.string "follower_uid", null: false
t.string "follower_type", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["followed_uid", "followed_type"], name: "index_follows_on_followed_uid_and_followed_type"
t.index ["follower_uid", "follower_type"], name: "index_follows_on_follower_uid_and_follower_type"
end
# Ransack query
relation = Task.all
search = relation.ransack({ users_uid_not_in: ['uid_example'] })
search.result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, some unexpected development get me away from this. I plan to look over early next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eqdrs check my latest updates?
4d30e0a
to
a402a20
Compare
59a5d65
to
2f74515
Compare
2f74515
to
86396d3
Compare
Fix nested conditions, polymorphic associations handling
This PR addresses several issues with Ransack's handling of polymorphic associations and negative predicates:
Key Improvements
Polymorphic Association Support: Added proper handling for polymorphic associations in correlated subqueries by correctly managing both foreign key and type columns.
Negative Predicate Handling: Fixed the behavior of negative predicates (
not_null
,not_cont
, etc.) when used with associations:not_null: true/false
now generates the correct IN/NOT IN queriesnot_cont
now properly generates NOT IN queries with LIKE conditionsComplex AND Conditions: Improved extraction of correlated keys from complex AND conditions with multiple children.
String Wildcard Handling: Added proper quoting for string values containing wildcards (%) in LIKE predicates.
Nested Condition Detection: Added helper method to correctly identify nested vs. non-nested conditions.
Tests Added
Other Changes
.idea
to.gitignore
This PR fixes issues with complex queries involving polymorphic associations and negative predicates, which previously generated incorrect SQL.
This PR incorporates changes from #1537 and #1279.