Fix for #19524 introduced a backward compatiblity issue with related managers on 1.5.X

[Simon Charette]

The fix for #19524 which was ​backported to the 1.5.X branch caused the following regression:

class ObjectQuerySet(models.query.QuerySet):
    def extra_qs_method(self):
        pass


class ObjectManager(models.Manager):
    use_for_related_fields = True

    def get_query_set(self):
        return ObjectQuerySet(self.model, using=self._db)


class RelatedObject(models.Model):
    pass


class Object(models.Model):
    related = models.ForeignKey(RelatedObject, related_name='objs')

    objects = ObjectManager()


RelatedObject().objs.extra_qs_method()

Raises

AttributeError: 'EmptyQuerySet' object has no attribute 'extra_qs_method'

It works perfectly on master since QuerySet.none() returns an instance of the correct class while setting it's underlying's query to empty and on < 1.5.X since prior to this backport there was no instance.pk ​check to return an EmptyQuerySet.

[Anssi Kääriäinen]

The likely fix is to do queryset.filter(pk=None) instead of queryset.none(). User visible effect is the same as what is now done in master.

[Aymeric Augustin]

Isn't it technically possible to have a nullable custom foreign key field? (There could be at most one object with a null pk.)

[Anssi Kääriäinen]

A pk can't be NULL, but if you use to_field, then you could have NULLs on both sides. We definitely want the semantics of SQL's JOIN ON (lhs.fk_val = rhs.to_field). This means that if either/both of those are NULL, then the result is "no match". (SQL's funny trinary boolean logic, NULL = NULL => UNKNOWN.)

If we go an try to change this we will have endless amount of problems.

[Aymeric Augustin]

Right, that makes sense.

[Michael Manfre]

To avoid the NULL = NULL issue, you could just go with a more explicit queryset.filter(Q(pk=1) & ~Q(pk=1)).

[Anssi Kääriäinen]

The correct approach seems to be to filter using pk__in=[] which ensures no query is executed.

I am too tired to write a patch today. If anybody else wants to tackle this I won't object...

[Simon Charette]

I'll tackle it.

[Simon Charette]

Added a patch based on stable/1.5.x with fix and failing testcase. It looks like the regression was introduced by ​the fix for #17541 after all.

I don't know if we should also add the tests to the master branch.

[Anssi Kääriäinen]

Every use of .none() pre-1.6 will cause this same bug. This is a "feature" of using .none(), and the reason .none() was changed in 1.6. Still, I think it is a good idea to fix this one as this is a regression. Existing code will fail, and the fix is trivial.

I will commit the patch to 1.5.

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

In f4132140f52c88b67d11743d4062a9d455959ffc:

[1.5.x] Fixed #19652 -- Fixed .none() regression in related fields

The regression was caused by using .none() when querying for related
models, and the origin field's value was None. This resulted in missing
custom related manager subclass as .none() returns plain QuerySet.

This isn't backport from master, in master .none() correctly preserves
the queryset's class.

Patch provided by Simon Charette, with some minor polish by committer.

[Aymeric Augustin <aymeric.augustin@…>]

In 9328ef0e84db2559af56d24c40e6b24b74e29a6f:

[1.5.x] Avoided a possible regression in 5097d3c5.

QuerySet.none() returns an instance of EmptyQuerySet, which may have
undesirable side effects in the presence of custom query set classes.

The implementation of .none() was refactored in master to have the same
effect as .filter(pkin=[]).

Refs #19652.

Thanks Simon Charrette for the report.

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

In f4132140f52c88b67d11743d4062a9d455959ffc:

[1.5.x] Fixed #19652 -- Fixed .none() regression in related fields

The regression was caused by using .none() when querying for related
models, and the origin field's value was None. This resulted in missing
custom related manager subclass as .none() returns plain QuerySet.

This isn't backport from master, in master .none() correctly preserves
the queryset's class.

Patch provided by Simon Charette, with some minor polish by committer.

[Aymeric Augustin <aymeric.augustin@…>]

In 9328ef0e84db2559af56d24c40e6b24b74e29a6f:

[1.5.x] Avoided a possible regression in 5097d3c5.

QuerySet.none() returns an instance of EmptyQuerySet, which may have
undesirable side effects in the presence of custom query set classes.

The implementation of .none() was refactored in master to have the same
effect as .filter(pkin=[]).

Refs #19652.

Thanks Simon Charrette for the report.