Boolean filter "field__isnull=False" not working with ForeignKey with to_field

[weidwonder]

models Manager:

class ModelManager(models.Manager):
    def get_queryset(self):
        return super(ModelManager, self).get_queryset().filter(parent__isnull=False)

models:

class Category(models.Model):
    parent = models.ForeignKey('Category', to_field='slug', db_column='parent',
                               max_length=32, blank=True, null=True,
                               related_name='models')
    name = models.CharField(max_length=50, blank=True, null=True)
    ....
    vehicle_models = ModelManager()

    class Meta:
        db_table = 'open_category'
        ordering = ['pinyin']

query:

Category.vehicle_models.filter(slug=model_slug).values('name').query

query's sql output is:

SELECT `open_category`.`name` FROM `open_category` WHERE (`open_category`.`parent` IS NULL AND `open_category`.`slug` = "suteng") ORDER BY `open_category`.`pinyin` ASC

it should be open_category.parent IS NOT NULL, I downgrade django to 1.9.9, it works.

[Baptiste Mispelon]

Hi,

Using the attached testcase based on your description, I managed to confirm the issue and tracked it down to commit 388bb5bd9aa3cd43825cd8a3632a57d8204f875f.

Thanks.

[Baptiste Mispelon]

Reproduction testcase (simplified)

[Baptiste Mispelon]

I simplified the testcase a bit to get rid of some "moving parts".

The key to the failure seems to be the to_field attribute of the ForeignKey. Without it, the query seems to be generated correctly.

--Edit--

Following the advice of kezabelle on IRC, I placed a PDB trace right before ​https://github.com/django/django/blob/master/django/db/models/lookups.py#L436 and found out that at this point, the value of self.rhs is 'False', that is the string 'False' rather than a boolean object, which triggers the wrong branch of the if statement.

--Edit 2--
Digging a bit deeper, it turns out that this line is where False gets converted to 'False': ​https://github.com/django/django/blob/master/django/db/models/fields/related_lookups.py#L100

[Matthew Wilkes]

I don't believe that IsNull lookups should be converting their values to the type of the parent field, however the RelatedLookupMixin has useful looking code for ensuring that MultiColSource is handled correctly.

Would the fix for this be as simple as:

class RelatedIsNull(RelatedLookupMixin, IsNull):
    def get_prep_lookup(self):
        return self.rhs

?

[Simon Charette]

Replying to MatthewWilkes:

I don't believe that IsNull lookups should be converting their values to the type of the parent field, however the RelatedLookupMixin has useful looking code for ensuring that MultiColSource is handled correctly.

Would the fix for this be as simple as:

class RelatedIsNull(RelatedLookupMixin, IsNull):
    def get_prep_lookup(self):
        return self.rhs

?

return super(RelatedLookupMixin, self).get_prep_lookup() would be more appropriate.

[Claude Paroz]

​PR resulting of discussions with bmispelon on IRC.

[Claude Paroz <claude@…>]

In 272eccf7:

Fixed #26983 -- Fixed isnull filtering on ForeignKey with to_field

Thanks weidwonder for the report.

[Claude Paroz <claude@…>]

In 6757c946:

[1.10.x] Fixed #26983 -- Fixed isnull filtering on ForeignKey with to_field

Thanks weidwonder for the report.
Backport of 272eccf7ff0ced609e5a5e3bb6b4a40e773e0e66 from master.

[Chris Lamb]

This also happens for field_isnull=False where field is a ForeignKey to a model with a CharField that is primary_key=True. Again, self.rhs is converted from False to 'False, ie a bool to a str`. Reopening as the pach does not fix this.

[Chris Lamb]

After writing a testcase (available here: ​https://gist.github.com/lamby/394b712e9e9ff1e868a20e67d4ba482b/raw) "git bisect" told me that it was indeed fixed with the above change. Investigating more, what happened was that I was applying the patch against RelatedIninstead of RelatedLookupMixin. Apologies for the noise there.

However, I am still correct in that the bug is not strictly to_field specific as it affects this primary_key case. Therefore, I suggest:

• You update the release documentation, etc. to reflect this
• You add my test (or similar) to prevent a regression

In light of this, I am not resolving this ticket so that these action items do not get overlooked.

[Tim Graham]

Could you send a pull request with those items? You can use "Refs #26983 --" in the commit message.

[Chris Lamb]

Replying to timgraham:

Could you send a pull request with those items? You can use "Refs #26983 --" in the commit message.

​https://github.com/django/django/pull/7104

[Tim Graham <timograham@…>]

In 9751326:

Refs #26983 -- Added test for isnull lookup to CharField with primary_key=True.

[Tim Graham <timograham@…>]

In c24a47b3:

[1.10.x] Refs #26983 -- Added test for isnull lookup to CharField with primary_key=True.

Backport of 97513269d73520d10722bbd10404be6ac4d48d07 from master