Opened 7 years ago

Closed 3 years ago

#11521 closed Bug (fixed)

QuerySet.update() doesn't accept fkey assignment by id (with patch)

Reported by: Jerome Leclanche Owned by: Jerome Leclanche
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: adys.wh@…, timograham@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

QuerySet.update doesn't accept FIELD_id assignments for ForeignKeys. This prevents the user from using the same dict to create or update a key.

Attached patch solves that problem by adding attname keys in the field cache. It may or may not be a hack.

Attachments (1)

fkeyassignbyid.patch (608 bytes) - added by Jerome Leclanche 7 years ago.
Add attname to field cache

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by Jerome Leclanche

Attachment: fkeyassignbyid.patch added

Add attname to field cache

comment:1 Changed 7 years ago by Jerome Leclanche

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Jerome Leclanche
Patch needs improvement: unset

comment:2 Changed 7 years ago by Jerome Leclanche

Ping. Any feedback on this?

comment:3 Changed 7 years ago by Russell Keith-Magee

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:4 Changed 5 years ago by Julien Phalip

Severity: Normal
Type: Bug

comment:5 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 Changed 3 years ago by Tim Graham

Cc: timograham@… added
Needs tests: unset

comment:8 Changed 3 years ago by Anssi Kääriäinen

As discussed in the pull request the change done to Options.init_name_map will effectively turn the allow_explicit_fk parameter useless. The change done is adding field.attname to the name map. The allow_explicit_fk parameter is used in join generation inside the sql/query.py, and it allows using attname for lookups when the parameter is set. The parameter is set for filter() and values() calls at least, but isn't set for some other operations, like annotate(). I can't see any reason for the inconsistency - either allow attname everywhere or nowhere.

One test fails if the change to init_name_map and removal of allow_explicit_fk is done. The failing test is lookup.tests.LookupTests.test_error_messages, it fails because author_id is in the error message's "allowed choices" list in addition to author. But, that is actually correct, author_id is an allowed choice.

Any opposition for making this change at alpha stage? There is a minor risk this could break something.

The proposed change will also fix this ticket's issue. A patch is available from: https://github.com/akaariai/django/compare/ticket_11521

comment:9 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

We can make this change for 1.7. Patch still applies cleanly and tests pass.

comment:10 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

Resolution: fixed
Status: newclosed

In e01b5a5823fa06a63382f87472978a16c77048d2:

Fixed #11521 -- usage of field.attname in .update()

Fixed already by previous patch, only test added.

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