Ransack: Rails 5.2.1: Join type for nested relation becomes INNER, not LEFT OUTER

[yhatt]

I have tried to upgrade own Rails from 5.2.0 to 5.2.1, but several tests using ransack query to nested relation have failed because of a join type difference.

A simple example to reproduce:

class A < ApplicationRecord
  belongs_to :b
end

class B < ApplicationRecord
  has_many :as
  has_many :cs
end

class C < ApplicationRecord
  belongs_to :b
end

# Search from A's related text `bs`.`name` or `cs`.`name`
A.ransack({ b_name_or_b_c_name_cont: 'foobar' }).result.to_sql

# Rails 5.2.0:
# SELECT "as".* FROM "as" LEFT OUTER JOIN "bs" ON "bs"."id" = "as"."b_id" LEFT OUTER JOIN "cs" ON "cs"."b_id" = "bs"."id" WHERE ("bs"."name" LIKE '%foobar%' OR "cs"."name" LIKE '%foobar%')
#                                                                         ^^^^^^^^^^
# Rails 5.2.1:
# SELECT "as".* FROM "as" LEFT OUTER JOIN "bs" ON "bs"."id" = "as"."b_id" INNER JOIN "cs" ON "cs"."b_id" = "bs"."id" WHERE ("bs"."name" LIKE '%foobar%' OR "cs"."name" LIKE '%foobar%')
#                                                                         ^^^^^

In Rails 5.2.1 and Ransack 2.0.1, the search query for B's name will never match if C was not related to any records from B.

I don't know which one is the correct query on Rails 5.2.1, but it is sure that is losing compatible with the previous version.

[ozzyaaron]

I find it hard to say anything for certain, BUT this appears to certainly be a bug. In our case we're doing a _not_null ransack query and so it cannot use an inner join for this type of query and possibly be correct as the last part is a WHERE check for NULLs and the INNER JOIN will not return records at all in this case.

class SearchIndex
  belongs_to :application
end

class Application
  has_one :instruction
end

SearchIndex.ransack(application_instructions_id_null: true).result.to_sql

SELECT "search_indices".*
FROM "search_indices"
LEFT OUTER JOIN "applications"
  ON "applications"."id" = "search_indices"."application_id"
INNER JOIN "instructions"
  ON "instructions"."application_id" = "applications"."id"
WHERE "instructions"."id" IS NULL

As you can see the INNER JOIN between applications and instructions is wrong and should be a LEFT OUTER JOIN as it was in Rails 5.2.0.

[igor04]

it looks like with AR 5.2.1 and INNER JOIN instead of LEFT OUTER there is no way to have valid search results with blank associations, because inner join will reject all results without association no matter that in condition we use or, ex: own_field_or_association_field_coun rejects records without association even if own_field matches (works fine with AR 5.2.0)

[seanfcarroll]

A PR with some failing tests would be super-helpful on this one.

[varyform]

This seems to help: https://github.com/activerecord-hackery/ransack/compare/master...back2war:fix/rails-522

We'll battle test on prod tomorrow :)

[nguyenductung]

@varyform How was your test? Do you plan to create a PR in this repository?

[dougc84]

I'm updating an app from Rails 4 to 5.2.x, thus requiring an update to Ransack 2.x. I can confirm @varyform's linked branch does seem to fix issues with improper joins (cleaning up about 40 failures) via that app's test suite.

[falcon000]

Hi guys,

It seems there is a fix for this issue. Do you plan to release it in near future?

[gregmolnar]

Anyone who had this issue, could you please verify that this PR fixes it? https://github.com/activerecord-hackery/ransack/pull/1004

[yhatt]

@gregmolnar I thank for your great work.

Failed specs caused by join type in our app have passed with updated Rails 5.2.2 by using your branch. I also confirmed that a search query will match to nested attribute via LEFT JOIN correctly.

On the other hand, a simple SQL query seems to have broken join type by patching. e.g. AR's left_joins becomes INNER JOIN.

class Foo < ApplicationRecord
  has_one :bar
end

class Bar < ApplicationRecord
  belongs_to :foo
end

# ✅ransack 2.1.1 & Rails 5.2.2
[1] pry(main)> Foo.left_joins(:bar).to_sql
=> "SELECT `foos`.* FROM `foos` LEFT OUTER JOIN `bars` ON `bars`.`foo_id` = `foos`.`id`"
                                ^^^^^^^^^^^^^^^
# gregmolnar/ransack:fix-polymorphic-joins & Rails 5.2.2
[2] pry(main)> Foo.left_joins(:bar).to_sql
=> "SELECT `foos`.* FROM `foos` INNER JOIN `bars` ON `bars`.`foo_id` = `foos`.`id`"
                                ^^^^^^^^^^

[gregmolnar]

@yhatt thanks! I will look into it. It looks like our specs have some issues:

(byebug) expect(subject.send(:join_root).drop(1).map(&:join_type)).to be_all { Polyamorous::OuterJoin }
true
(byebug) subject.send(:join_root).drop(1).map(&:join_type)
[Arel::Nodes::OuterJoin, Arel::Nodes::InnerJoin]

That be_all matcher seems to be broken.

[gregmolnar]

@yhatt Could you please check your app against this PR again? https://github.com/activerecord-hackery/ransack/pull/1004

[yhatt]

Sorry for the delay in responding. I've tested updated PR again the all of failed tests in my app have passed as like as Rails <= 5.2.0. It seems to be fixed the wrong join type too. Thanks for your excellent work against so deep problem!

[gregmolnar]

This one is resolved now I think.