Opened 8 years ago

Last modified 2 years 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
Pull Requests:7841 unmerged

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.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (6)

comment:1 by Tim Graham, 8 years ago

Does the test suite pass with the change?

comment:2 by Tim Graham, 8 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, 8 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, 8 years ago

Resolution: needsinfo
Status: closednew

comment:5 by Tim Graham, 8 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, 8 years ago

Has patch: unset

The pull request has been withdrawn.

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