select_related and annotate won't follow nullable foreign keys

[Eivind Fonn <evfonn@…>]

To reproduce, first the models:

from django.db import models

class Alfa(models.Model):
    pass

class Bravo(models.Model):
    pass

class Charlie(models.Model):
    alfa = models.ForeignKey(Alfa, null=True)
    bravo = models.ForeignKey(Bravo, null=True)

Then in shell:

In [2]: b = Bravo.objects.create()

In [3]: c = Charlie.objects.create(bravo=b)

In [4]: qsboth = Charlie.objects.select_related('alfa').annotate(Count('bravo__charlie')); qsboth
Out[4]: []

In [5]: qsselrel = Charlie.objects.select_related('alfa'); qsselrel
Out[5]: [<Charlie: Charlie object>]

In [6]: qsanno = Charlie.objects.annotate(Count('bravo__charlie')); qsanno
Out[6]: [<Charlie: Charlie object>]

As you can see, select_related and annotate both work as expected, but not together. Queries:

In [7]: print(str(qsboth.query))
SELECT "bugrep_charlie"."id", "bugrep_charlie"."alfa_id", "bugrep_charlie"."bravo_id", COUNT(T4."id") AS "bravo__charlie__count", "bugrep_alfa"."id" FROM "bugrep_charlie" INNER JOIN "bugrep_alfa" ON ( "bugrep_charlie"."alfa_id" = "bugrep_alfa"."id" ) LEFT OUTER JOIN "bugrep_bravo" ON ( "bugrep_charlie"."bravo_id" = "bugrep_bravo"."id" ) LEFT OUTER JOIN "bugrep_charlie" T4 ON ( "bugrep_bravo"."id" = T4."bravo_id" ) GROUP BY "bugrep_charlie"."id", "bugrep_charlie"."alfa_id", "bugrep_charlie"."bravo_id", "bugrep_alfa"."id"

In [8]: print(str(qsselrel.query))
SELECT "bugrep_charlie"."id", "bugrep_charlie"."alfa_id", "bugrep_charlie"."bravo_id", "bugrep_alfa"."id" FROM "bugrep_charlie" LEFT OUTER JOIN "bugrep_alfa" ON ( "bugrep_charlie"."alfa_id" = "bugrep_alfa"."id" )

Using only select_related yields an outer join on the alfa table, while adding annotate in the mix gives an inner join. Indeed, if we make an alfa object, it works fine:

In [9]: a = Alfa.objects.create()

In [10]: Charlie.objects.update(alfa=a)
Out[10]: 1

In [11]: Charlie.objects.annotate(Count('bravo__charlie')).select_related('alfa')
Out[11]: [<Charlie: Charlie object>]

[Eivind Fonn <evfonn@…>]

Shell session to reproduce

[Eivind Fonn <evfonn@…>]

Quick executable to reproduce (assumes project and app are called bugrep)

[Baptiste Mispelon]

The issue seems to have been fixed in master by commit c21e86ab9e3e5ebd6d245d038cb0cb352cd84c3a.

I'm marking it as a regression since it's not present in 1.5 either (it was introduced by edf93127bf2f9dc35b45cdea5d39a1b417ab1638).

Thanks for the detailed report.

[Anssi Kääriäinen]

It seems the problem is that setup_joins() arguments have changed. It is called with positional arguments in many places, and as the arguments for setup_joins() change, the flags passed to setup_joins() as positional arguments end up targeting different parameters.

I recommend to go through all uses of setup_joins() in the ORM, and change the calls to use kwarg=someval for all optional arguments. Otherwise further changes will again cause similar regressions.

The patch doesn't need to be backpatched, just checking that correct arguments are passed to setup_joins() is enough. Quickly thinking I am not sure if backpatching could cause further regressions. Seems safe to backpatch, but I can't guarantee it is actually safe to do so.

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

In 1a922870ea07e730281c0e2a7b3a170232a81236:

[1.6.x] Fixed #21150 -- Improved Query.add_fields() join promotion logic

Thanks to Eivind Fonn for the report and test case.

[Anssi Kääriäinen]

While working on adding tests for this ticket in master I noticed a couple of possibilities for improved join promotion logic. The PR at ​https://github.com/django/django/pull/1677 can now do join demotion (that is, joins that are promoted to outer joins earlier in the query can be demoted back to inner joins later on), and some cases of annotate() and .values() will create correctly inner joins instead of outer join.

There are a couple of added tests for .annotate() and .values() cases, and a couple of expectedFailures are solved by this patch.

I wonder if the added comment for promote_filter_joins() makes any sense to anybody else than me. If not, please notify me.

I have added comments explaining the changes done to the PR. To me it seems a good way to explain why things are changed the way they are changed...

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

In ed0d720b78c8f1c655ead0057d767a0712f1a6a8:

Fixed #21150 -- select_related + annotate join promotion failure

Added tests for a .annotate().select_related() join promotion failure.
This happened to work on master but was currently untested.

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

In ecaba3602837d1e02fe1e961f7d3bf9086453259:

Improved Query join promotion logic

There were multiple cases where join promotion was a bit too aggressive.
This resulted in using outer joins where not necessary.

Refs #21150.