Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#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 Changed 3 years ago by Tim Graham

Summary: update_or_create silently ignores mismatching field names in defaultsMake QuerySet.get_or_create()/update_or_create() error for a non-field in defaults
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

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.

comment:2 Changed 3 years ago by François Freitag

Has patch: set
Last edited 3 years ago by Tim Graham (previous) (diff)

comment:3 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In a5e13a0b:

Fixed #27118 -- Made QuerySet.get_or_create()/update_or_create() error for a non-field in their arguments.

comment:4 Changed 3 years ago by Tim Graham

Cc: François Freitag added
Has patch: unset
Keywords: 1.11 added
Resolution: fixed
Status: closednew

The fix needs some adjustment to allow using pk=# as a field, which is a valid use case.

comment:5 Changed 3 years ago by François Freitag

Has patch: set

comment:6 Changed 3 years ago by Tim Graham <timograham@…>

In 1db1f74:

Refs #27118 -- Reallowed using pk in QuerySet.get/update_or_create().

comment:7 Changed 3 years ago by Tim Graham

Resolution: fixed
Status: newclosed

comment:8 Changed 2 years ago 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

comment:9 Changed 2 years ago 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.

Note: See TracTickets for help on using tickets.
Back to Top