LOUTER join not promoted across filter expression

[milosu]

In my database I have a little bit complicated relations. The attached test case fails without applied patch.

The core of the problem is actually something like:

ModelA <-- ModelB ---0--> Model C --> Model D
                  <--0--> Model C --> Model D

When I put ModelA into django.contrib.admin changelist and try to search - the search fields containing fields from all above outlined models, the resulting queryset contains one INNER JOIN for the link between the second relation of Model C and Model D which removes from the result set those models for which the many2many link is empty - which I guess should not happen.

I think that the LEFT OUTER JOIN should propagage in this case across relations. Maybe this patch solves also other ORM OR-queries related problems present or already patched earlier..

The attached patch solves this problem and I hope it passes the whole Django test suite - although due to the time limitations I tried it only with "queries" and "aggregation_regress tests". With this patch I was running my app in production for 2 years now, so I expect it *should* work.

This bug was present in the Django ORM since the qs refactor branch was merged a few years ago so I think it does not need to be marked as 1.4 release bloker. But anyway..

[Aymeric Augustin]

The explanation makes sense to me.

But I'm not good enough with SQL and the ORM to guarantee that we want this change.

Also I'm a bit worried about backwards compatibility, since I can't tell exactly what this change would affect.

[Anssi Kääriäinen]

The idea in the patch seems correct to me. Closely related to #16715 and #10790.

However, the patch isn't to proper level (it is to +++ ../Django-1.4c1/django/db/models/sql/query.py), and there are stylistic errors (tab before space, missing newline between the test method at least).

I hope you can reformat the patch (using git is recommended) and repost it or link to github. If not, I think I will do that anyways, as to me it seems this patch is needed alongside #16715 for correct join promotions.

[milosu]

polished patch

[Anssi Kääriäinen]

I now know what was the reason for the wrong query. The basic problem was that we had an existing LOUTER JOIN, and joined a non-nullable FK to that. In that case we must LOUTER JOIN the new join, too, but the current code doesn't consider the case where the previous join was a LOUTER JOIN. The patch just directly creates the join as LOUTER JOIN, thus there is no need to fix promote_alias() logic.

#16715 would fix this issue, but even if I am planning to commit that one soon, I do think the idea of generating LOUTER JOIN automatically in case we are joining to an existing LOUTER JOIN chain is obviously correct, and thus it is a good idea to apply this.

The cleaned up patch is ​here, the main change is that the test case is reduced to the minimal set of filters displaying the issue in this ticket.

[Anssi Kääriäinen <akaariai@…>]

In [a193372753ad9d1d15ad5e2d6d06bbc07ca3f433]:

Fixed #17886 -- Fixed join promotion in ORed nullable queries

The ORM generated a query with INNER JOIN instead of LEFT OUTER JOIN
in a somewhat complicated case. The main issue was that there was a
chain of nullable FK -> non-nullble FK, and the join promotion logic
didn't see the need to promote the non-nullable FK even if the
previous nullable FK was already promoted to LOUTER JOIN. This resulted
in a query like a LOUTER b INNER c, which incorrectly prunes results.