Fix `not_in` search with deep nesting
Description
This pull request addresses issue #1411, where the generated SQL query for excluding articles based on associated tags was incorrect (I've used tags the my example).
The current query joins and filters tags improperly, resulting in an unintended query structure. Specifically, the subquery logic used for filtering tags produces incorrect results and may impact performance.
SELECT "articles".*
FROM "articles"
LEFT OUTER JOIN "comments"
ON "comments"."disabled" = 0
AND "comments"."article_id" = "articles"."id"
WHERE ('default_scope' = 'default_scope')
AND "articles"."id" NOT IN (
SELECT "comments_tags"."comment_id"
FROM "comments_tags"
INNER JOIN "tags"
ON "tags"."id" = "comments_tags"."tag_id"
WHERE "comments_tags"."comment_id" = "articles"."id"
AND NOT ("tags"."name" NOT IN ('Fantasy', 'Scifi'))
)
The solution modifies the query structure to use proper joins and filtering, simplifying the SQL and ensuring correct results.
SELECT "articles".*
FROM "articles"
LEFT OUTER JOIN "comments"
ON "comments"."disabled" = 0
AND "comments"."article_id" = "articles"."id"
LEFT OUTER JOIN "comments_tags"
ON "comments_tags"."comment_id" = "comments"."id"
LEFT OUTER JOIN "tags"
ON "tags"."id" = "comments_tags"."tag_id"
WHERE ('default_scope' = 'default_scope')
AND "tags"."name" NOT IN ('Fantasy', 'Scifi')
@clgiovannelli I've incorporated your changes into #1561 as I'm dealing with the complex scenario that requires multiple interconnected changes from your PR, #1279, and my own to be applied at the same time. I also added additional test coverage. Tell me if you're ok with that.
@clgiovannelli I've incorporated your changes into #1561 as I'm dealing with the complex scenario that requires multiple interconnected changes from your PR, #1279, and my own to be applied at the same time. I also added additional test coverage. Tell me if you're ok with that.
Thank you for taking the time to look into this. I'll check your changes today.
@giovannelli can this be closed now https://github.com/activerecord-hackery/ransack/pull/1561 is merged?
@giovannelli can this be closed now #1561 is merged?
Thank you!
@giovannelli can this be closed now #1561 is merged?
Thank you!
Great! Thank you for getting back to me @giovannelli 🤝