[Django] #29727: Lookup using F() on a non-existing model field doesn't throw an error
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
vidyarani-dg |
Type: Bug | Status: new
Component: Database | Version: 2.1
layer (models, ORM) |
Severity: Normal | Keywords: F() expressions
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Fetching the value of a non-existing field on a model using F() object
does not throw an error.
Instead, it is just giving the FK value of the instance.
Please find below a minimal test case to reproduce this issue.
{{{
#!python
# lineitem/models.py
from django.db import models
class Account(models.Model):
REGULAR = 'R'
GOLD = 'G'
PLATINUM = 'P'
ACCOUNT_TYPE_CHOICES = (
(REGULAR, 'Regular'),
(GOLD, 'Gold'),
(PLATINUM, 'Platinum'),
)
account_type = models.CharField(
max_length=1,
choices=ACCOUNT_TYPE_CHOICES,
default=REGULAR,
)
class Client(models.Model):
name = models.CharField(max_length=50)
registered_on = models.DateField()
account_type = models.ForeignKey(Account, on_delete=models.CASCADE)
$ python manage.py shell
In [1]: from datetime import date, timedelta
In [2]: from models import Client, Account
In [3]: Client.objects.create(
...: name='Jane Doe',
...:
account_type=Account.objects.create(account_type=Account.REGULAR),
...: registered_on=date.today() - timedelta(days=36)
...: )
...: Client.objects.create(
...: name='James Smith',
...:
account_type=Account.objects.create(account_type=Account.GOLD),
...: registered_on=date.today() - timedelta(days=5)
...: )
...: Client.objects.create(
...: name='Jack Black',
...:
account_type=Account.objects.create(account_type=Account.PLATINUM),
...: registered_on=date.today() - timedelta(days=10 * 365)
...: )
...:
...:
Out[3]: <Client: Client object (6)>
In [4]: from django.db.models import F
In [5]: Client.objects.values(client_name=F('name'),
account_name=F('account_type__name'))
Out[5]: <QuerySet [{'client_name': 'Jane Doe', 'account_name': 2},
{'client_name': 'James Smith', 'account_name': 3}, {'client_name': 'Jack
Black', 'account_name': 4}, {'client_name': 'Jane Doe', 'account_name':
5}, {'client_name': 'James Smith', 'account_name': 6}, {'client_name':
'Jack Black', 'account_name': 7}]>
In [6]: print(queryset.query)
SELECT "lineitem_client"."name" AS "client_name",
"lineitem_client"."account_type_id" AS "account_name" FROM
"lineitem_client"
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29727>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: F() expressions | Triage Stage:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
I tried this in Django 1.11, and it raises the expected exception.
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:1>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:2>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "test_bogus_lookup.py" added.
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
Git bisect tells me it's:
{{{
2162f0983de0dfe2178531638ce7ea56f54dd4e7 is the first bad commit
commit 2162f0983de0dfe2178531638ce7ea56f54dd4e7
Author: Matthew Wilkes <g...@matthewwilkes.name>
Date: Sun Jun 18 16:53:40 2017 +0100
Fixed #24747 -- Allowed transforms in QuerySet.order_by() and
distinct(*fields).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:3>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
https://github.com/django/django/commit/2162f0983de0dfe2178531638ce7ea56f54dd4e7
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:4>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
So, does it think it's a transform?
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:5>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Curtis Maloney):
Even if it does, it should surely raise an error? I don't know of a
transform called "name", do you?
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:6>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Vidya Rani D G):
I tried this in Django 2.0 and it raises an exception.
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:7>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
Replying to [comment:7 Vidya Rani D G]:
> I tried this in Django 2.0 and it raises an exception.
Okay, that's weird, because the test I wrote passed under 2.0
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:8>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alexander Holmbäck):
Added a test expecting FieldError when F() contains a join that ends with
something other than a field or transform (test fails in 2.1a1 and above):
https://github.com/alexh546/django/tree/ticket_29727
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:9>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Alexander Holmbäck
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:10>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:11>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alexander Holmbäck):
Here's my take
In `django.db.models.sql.Query`:
`setup_joins` holds on to `FieldException` raised by `names_to_path` so
that `JoinInfo.transform_function` can reraise if it doesn't find a
satisfying transform.
It seems like `resolve_ref` doesn't need the return value from
`transform_function` (unlike `add_fields`), so it didn't call it, which
accidentally led `FieldError` to never be reraised.
When adding a call to `transform_function` from `resolve_ref`, all tests
passes. Note how I assume `targets` to not be empty, that may be bad.
Also, I haven't confirmed that there are tests for when joins and
transforms are mixed, which could be relevant but also not.
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:12>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hi Alexander. Can you open
[https://github.com/django/django/compare/master...alexh546:ticket_29727 a
PR from your branch to master]? Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:13>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alexander Holmbäck):
Done.
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:14>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: F() expressions | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* severity: Normal => Release blocker
Comment:
IMO it should be marked as a "Release Blocker", because it is a regression
in the 2.1 release.
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:15>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: felixxm (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:16>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:17>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: F() expressions | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f315d0423a09dfe20dd4d4f6a0eb11fc8e45a665" f315d042]:
{{{
#!CommitTicketReference repository=""
revision="f315d0423a09dfe20dd4d4f6a0eb11fc8e45a665"
Fixed #29727 -- Made nonexistent joins in F() raise FieldError.
Regression in 2162f0983de0dfe2178531638ce7ea56f54dd4e7.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:18>
Django
-------------------------------------+-------------------------------------
| Holmbäck
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: F() expressions | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"bd5ce0599bf0dc156b943ca0d03307c7e451923c" bd5ce059]:
{{{
#!CommitTicketReference repository=""
revision="bd5ce0599bf0dc156b943ca0d03307c7e451923c"
[2.1.x] Fixed #29727 -- Made nonexistent joins in F() raise FieldError.
Regression in 2162f0983de0dfe2178531638ce7ea56f54dd4e7.
Backport of f315d0423a09dfe20dd4d4f6a0eb11fc8e45a665 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29727#comment:19>