#27118 closed Cleanup/optimization (fixed)
Make QuerySet.get_or_create()/update_or_create() error for a non-field in defaults
Reported by: | Tom Brightwell | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.9 |
Severity: | Normal | Keywords: | 1.11 |
Cc: | François Freitag | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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)
Change History (9)
comment:1 by , 8 years ago
Summary: | update_or_create silently ignores mismatching field names in defaults → Make QuerySet.get_or_create()/update_or_create() error for a non-field in defaults |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:4 by , 8 years ago
Cc: | added |
---|---|
Has patch: | unset |
Keywords: | 1.11 added |
Resolution: | fixed |
Status: | closed → new |
The fix needs some adjustment to allow using pk=#
as a field, which is a valid use case.
comment:5 by , 8 years ago
Has patch: | set |
---|
comment:7 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:8 by , 8 years ago
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
comment:9 by , 8 years ago
I'm not sure if we'd fix that, but please open a new ticket with details rather than commenting on a closed ticket.
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.