fix polymorphic joins with rails > 5.2

[gregmolnar]

closes #996 and #902 and #955
I still need to add some specs to cover this.

[dholdren]

I was able to confirm that this solves my issue. Thanks again.

[rzane]

@gregmolnar, I just ran this against baby_squeel's test suite (branch here, results here).

It seems like this does fix #902 and #955, so great work. It fixed all of the tests that were specifically related to polymorphism. Those tests had been broken in baby_squeel since Active Record 5.2 initially came out.

However, #955 seems to be in play still. You'll notice that some of the tests that should say LEFT JOIN are instead saying INNER JOIN.

The branch also includes this change as a workaround for the fact that JoinAssociation#table seems to be returning nil sometimes. The change seems to fix most of the tests, but I suspect that it might be covering up a larger issue. I suspect that the workaround is also causing the test failures that are reporting the wrong table alias. Maybe this is a clue?

[gregmolnar]

@rzane Thanks for the info. I need to find a few hours block when I go over again this and find the root cause.

[kieranklaassen]

Thanks, guys for the work on this! I would really appreciate it if you can merge this one @gregmolnar .

[gregmolnar]

@kieranklaassen the trouble is the fix on this branch causes issues if you use left_joins in you app. I know what causes the problem and have a rough idea of a fix, just need to find the time when I can sit down for a few hours and get it done. Hopefully I will manage to get there soon.

[kieranklaassen]

@gregmolnar I appreciate your efforts very much!! If you do not have time but could share your idea for the fix I might be able to help out here too. Thanks!

[pelletencate]

@gregmolnar I'd love to help out here too. We ourselves depend on baby_squeel, and it'd be awesome if I can be of assistance in speeding up this process a bit. If it's easier / less time consuming for you to do a brain-dump here, I'm more than happy to try and give it a go!

[gregmolnar]

I need to get into the code to get my head around the problem again to be able to make a brain-dump and I will probably just fix it if I manage to get there.
On a very high level, the issue is we used to override ActiveRecord::Associations::JoinDependency#make_left_outer_joins, but that is either removed on Rails 5.2, or we thought we don't need to override it anymore(I can't remember) but we still need a workaround to turn our joins into a left one. Beware, I am writing this without checking the codebase, but this is roughly where I remember to look where debugged this issue.

[gregmolnar]

I finally managed to work on this and I think it is fixed now, although I couldn't come up with specs so far.
Could you please guys verify that it works for you?
@rzane I ran the specs in your branch and some still fails, but those failures might be unrelated? I don't know much about baby_squeel, but it looks like the issue happens with some aliasing.

[gregmolnar]

@rzane Actually, those alias errors looks like caused by ransack too, and there is a PR to fix that: https://github.com/activerecord-hackery/ransack/pull/1005/files
I will merge this first if I get the feedback of it being ok, then will merge that too.

[rzane]

Thanks again for digging into this @gregmolnar.

It seems like once a relation starts to do any outer join, all subsequent joins become outer joins? Take this example:

Post.joining { author.outer.comments }

In the example above, I would expect posts -> author to be an outer join and author -> comments to be an inner join. In previous Active Record versions, that works as expected. But, that's not what is happening on 5.2.1. Here's the failing spec:

expected:
        SELECT "posts".* FROM "posts"
        LEFT OUTER JOIN "authors" ON "authors"."id" = "posts"."author_id"
        INNER JOIN "comments" ON "comments"."author_id" = "authors"."id"
     got:
        SELECT "posts".* FROM "posts"
        LEFT OUTER JOIN "authors" ON "authors"."id" = "posts"."author_id"
        LEFT OUTER JOIN "comments" ON "comments"."author_id" = "authors"."id"

Another issue is the alias detection. Here's a gist of what that looks like in Baby Squeel.

def find_alias(associations)
   join_association = find_join_association(associations)
   join_association.table
end

Baby Squeel is successfully finding the correct JoinAssociation. On previous versions of Active Record, #table would return an Arel::Table with the correct name. However, when running against 5.2.1, #table is nil. I'm honestly not sure if this part is a polyamorous issue or a baby_squeel issue caused by a change in Active Record.

[gregmolnar]

I merged this, since it fixes most of the issues and I want to make master work with Rails 6. I will open a follow up issue for the remaining issue.