[Django] #27118: update_or_create silently ignores mismatching field names in defaults
Django
----------------------------------------------+--------------------
Reporter: brighters | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
I'm not sure if this intended behaviour or an issue.
I came across this issue using the update_or_create method. I was passing
a set of defaults where one of the keys did not match with a field on the
model. When called in an update scenario the method worked and returned
the updated object and False, updating the model fields which matched with
the keywords provided in the defaults parameter. But in a create scenario
I got a "TypeError: '[incorrect_field_name]' is an invalid keyword
argument for this function.". My test coverage was a little lacking as it
only covered update scenarios, so I only came across the issue in
production in a create scenario.
I presume this is because the setattr method still runs successfully
against the model object, even if the keyword in defaults does not
correspond to a model field?
{{{
query.py
for k, v in six.iteritems(defaults):
setattr(obj, k, v)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27118>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
defaults
-------------------------------------+-------------------------------------
Reporter: brighters | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Raising an error will be backwards-incompatible, but I think it's more
likely to reveal bugs than to cause a problem for a legitimate use case.
If pre-release testing reveals otherwise, we could reconsider.
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:1>
Django
defaults
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by francoisfreitag):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:2>
Django
defaults
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"a5e13a0b926156f7885a0f1d8b300221f74601fc" a5e13a0b]:
{{{
#!CommitTicketReference repository=""
revision="a5e13a0b926156f7885a0f1d8b300221f74601fc"
Fixed #27118 -- Made QuerySet.get_or_create()/update_or_create() error for
a non-field in their arguments.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:3>
Django
defaults
-------------------------------------+-------------------------------------
Reporter: Tom Brightwell | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* keywords: => 1.11
* cc: François Freitag (added)
* has_patch: 1 => 0
* status: closed => new
* resolution: fixed =>
Comment:
The fix needs some adjustment to allow using `pk=#` as a field, which is a
valid use case.
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:4>
Django
defaults
-------------------------------------+-------------------------------------
Reporter: Tom Brightwell | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: 1.11 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by François Freitag):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:5>
Django
defaults
-------------------------------------+-------------------------------------
Reporter: Tom Brightwell | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: 1.11 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1db1f74617ae0c7b100065117ed1d9ac19a6143a" 1db1f74]:
{{{
#!CommitTicketReference repository=""
revision="1db1f74617ae0c7b100065117ed1d9ac19a6143a"
Refs #27118 -- Reallowed using pk in QuerySet.get/update_or_create().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:6>
Django
defaults
-------------------------------------+-------------------------------------
Reporter: Tom Brightwell | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Keywords: 1.11 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:7>
Django
defaults
-------------------------------------+-------------------------------------
Reporter: Tom Brightwell | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: 1.11 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Anthony King):
I believe this has introduced a regression, where by if you pass into
defaults a value that goes through a setter, a ValueError will be raised.
The behaviour of `.create` is to create a new instance of a Model, and let
the exception bubble up.
This isn't behaviour we rely on in our system anymore, but we have used it
in the past, and still do for `.create`
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:8>
Django
defaults
-------------------------------------+-------------------------------------
Reporter: Tom Brightwell | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: 1.11 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I'm not sure if we'd fix that, but please open a new ticket with details
rather than commenting on a closed ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/27118#comment:9>