Opened 9 years ago
Last modified 3 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 |
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 , 9 years ago
comment:2 by , 9 years ago
| Resolution: | → needsinfo |
|---|---|
| Status: | new → closed |
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 , 9 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.
comment:4 by , 9 years ago
| Resolution: | needsinfo |
|---|---|
| Status: | closed → new |
comment:5 by , 9 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: | Unreviewed → Accepted |
| Type: | Uncategorized → Cleanup/optimization |
Tentatively accepting, although I haven't analyzed this in detail. Perhaps adding a test to the pull request will clarify the use case.
Does the test suite pass with the change?