Skip to content
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

Added remove_association_no_negative_assoc to config to allow user to… #1123

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Doctor06
Copy link

… decide if they want to keep the join when using negative predicates.

Fix issue where negative predicates with associated_collection does not use the correct primary key for subquery.

… decide if they want to keep the join when using negative predicates.

Fix issue where negative predicates with associated_collection does not use the correct primary key for subquery.
@Doctor06
Copy link
Author

I had failing tests when i tried pulling from master without me making any changes. What do i do when that happens? Those checks that fails are the original failing tests i inherited from just forking.

@scarroll32
Copy link
Member

@Doctor06 please try rebasing against master.

 into fix_negative_predicate_primary_key_sub_query

# Conflicts:
#	lib/ransack/configuration.rb
#	spec/ransack/search_spec.rb
@scarroll32 scarroll32 mentioned this pull request Feb 24, 2021
13 tasks
@deivid-rodriguez
Copy link
Contributor

@Doctor06 Perhaps context has been lost here and you're no longer interested but master branch should be fully green now, so if you could rebase and fix merge conflicts that'd be awesome!

@scarroll32
Copy link
Member

@Doctor06 we would love to make this one of the changes released with the 3.0.0 release

@scarroll32
Copy link
Member

This is breaking the not_null predicate.

Removing from 3.0.0 release.

 1) Ransack::Adapters::ActiveRecord::Base negative conditions on HABTM associations removes redundant joins from top query
     Failure/Error: expect(sql).to_not include('LEFT OUTER JOIN')
       expected "SELECT \"articles\".* FROM \"articles\" WHERE ('default_scope' = 'default_scope') AND \"articles\".\...cles_tags\".\"tag_id\" WHERE ('default_scope' = 'default_scope') AND \"tags\".\"name\" = 'Fantasy')" not to include "LEFT OUTER JOIN"
     # ./spec/ransack/adapters/active_record/base_spec.rb:159:in `block (3 levels) in <module:ActiveRecord>'

  2) Ransack::Predicate not_null with association qeury generates a value IS NULL query when assigned false
     Failure/Error: expect(sql).to match /#{field} IS NULL/

       expected "SELECT \"people\".* FROM \"people\" WHERE \"people\".\"id\" NOT IN (SELECT \"people\".\"id\" FROM \"...RE \"comments\".\"id\" IS NOT NULL ORDER BY \"people\".\"id\" DESC) ORDER BY \"people\".\"id\" DESC" to match /"comments"."id" IS NULL/
       Diff:
       @@ -1 +1 @@
       -/"comments"."id" IS NULL/
       +"SELECT \"people\".* FROM \"people\" WHERE \"people\".\"id\" NOT IN (SELECT \"people\".\"id\" FROM \"people\" LEFT OUTER JOIN \"comments\" ON \"comments\".\"disabled\" = 0 AND \"comments\".\"person_id\" = \"people\".\"id\" WHERE \"comments\".\"id\" IS NOT NULL ORDER BY \"people\".\"id\" DESC) ORDER BY \"people\".\"id\" DESC"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants