Opened 7 years ago

Last modified 12 months ago

#27614 new Cleanup/optimization

Store the DB used in the state before calling Model._save_table()

Reported by: Joseph Kahn Owned by: nobody
Component: Database layer (models, ORM) Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I was trying to figure out why when calling .using before .create the self._state.db was not available. I happen to want the result of that to use in the pk value, so right now I have to re fetch that information. As it turns out, the cause can be narrowed down to a single line. However, I'm not sure if it was ordered that way for a reason but here:
https://github.com/django/django/blob/d2a26c1a90e837777dabdf3d67ceec4d2a70fb86/django/db/models/base.py#L831-L836

Since not every function call gets the using param passed to it, but most get the instance like when it wants to get a pk value prior to saving.

I was wondering if it's possible to move that self._state.db = using up.

Change History (6)

comment:1 by Tim Graham, 7 years ago

Does the test suite pass with the change?

comment:2 by Tim Graham, 7 years ago

Resolution: needsinfo
Status: newclosed

The proposed change doesn't seem to work, at least multiple_database.tests.RouterTestCase.test_deferred_models fails. Feel free to reopen if you have a working patch.

comment:3 by Joseph Kahn, 7 years ago

I created a PR with a change that doesn't break that test but fixes it for a lot of the edge cases I run into with multiple databases.

https://github.com/django/django/pull/7841

comment:4 by Joseph Kahn, 7 years ago

Resolution: needsinfo
Status: closednew

comment:5 by Tim Graham, 7 years ago

Has patch: set
Needs tests: set
Summary: Storing the DB used in the state before calling `_save_table`Store the DB used in the state before calling Model._save_table()
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Tentatively accepting, although I haven't analyzed this in detail. Perhaps adding a test to the pull request will clarify the use case.

comment:6 by Ingo Klöcker, 7 years ago

Has patch: unset

The pull request has been withdrawn.

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