[Django] #33124: Avoid accessing ConnectionsHandler.__getitem__ in a few more places until it's strictly necessary.

Django

unread,

Sep 21, 2021, 6:18:20 AM9/21/21

to django-...@googlegroups.com

#33124: Avoid accessing ConnectionsHandler.__getitem__ in a few more places until
it's strictly necessary.
-------------------------------------+-------------------------------------
Reporter: Keryn | Owner: Keryn Knight
Knight |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Follows on from #33025.

Most of the codebase does the correct thing as far as I can see,
preferring `conn(ection) = connections[alias]` and holding a reference to
that, when it's used multiple times. Most of the codebase also has
touching `connections[alias]` as the last component in `if` branches etc
too, so that short circuiting can occur to avoid touching the `Local`.

There's a few places where that isn't the case though. eg:
{{{
empty_strings_as_null =
connections[db].features.interprets_empty_strings_as_nulls
...
if val is None or (val == '' and empty_strings_as_null):
}}}
or
{{{
can_ignore_conflicts = (
connections[db].features.supports_ignore_conflicts and
self.through._meta.auto_created is not False
)
}}}
or
{{{
compiler = connections[db].ops.compiler('SQLCompiler')(
self.query, connections[db], db
)
}}}

Whilst I've not gone to the effort of benchmarking each of them, each of
them has the possibility to slightly improve things, I think.

I'll be pushing a PR shortly with a bunch of commits that cover all of
those I've found (outside of tests, where things are more gnarly and could
be tackled separately, if there's an impetus to do so), so that CI can
prove things work, and so that it can be decided which, if any of them,
are worth it.

--
Ticket URL: <https://code.djangoproject.com/ticket/33124>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Sep 21, 2021, 6:29:02 AM9/21/21

to django-...@googlegroups.com

#33124: Avoid accessing ConnectionsHandler.__getitem__ in a few more places until
it's strictly necessary.
-------------------------------------+-------------------------------------

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Keryn Knight):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1

Comment:

PR is https://github.com/django/django/pull/14876
Marking as patch needs improvement because it's a whole bunch of commits,
none with a 'proper' commit message, and also because we may not wish to
pull _all_ of the proposed changes in.

--
Ticket URL: <https://code.djangoproject.com/ticket/33124#comment:1>

Django

unread,

Sep 22, 2021, 1:42:34 AM9/22/21

to django-...@googlegroups.com

#33124: Avoid accessing ConnectionsHandler.__getitem__ in a few more places until
it's strictly necessary.
-------------------------------------+-------------------------------------

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/33124#comment:2>

Django

unread,

Sep 22, 2021, 1:43:12 AM9/22/21

to django-...@googlegroups.com

#33124: Avoid accessing ConnectionsHandler.__getitem__ until it's strictly
necessary.
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

--
Ticket URL: <https://code.djangoproject.com/ticket/33124#comment:3>

Django

unread,

Sep 24, 2021, 6:20:33 AM9/24/21

to django-...@googlegroups.com

#33124: Avoid accessing ConnectionsHandler.__getitem__ until it's strictly
necessary.
-------------------------------------+-------------------------------------

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/33124#comment:4>

Django

unread,

Sep 24, 2021, 12:41:36 PM9/24/21

to django-...@googlegroups.com

#33124: Avoid accessing ConnectionsHandler.__getitem__ until it's strictly
necessary.
-------------------------------------+-------------------------------------

Reporter: Keryn Knight | Owner: Keryn
Type: | Knight

Cleanup/optimization | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"06c50cee0fb1fd23248518334eabb2fbf61b2cbc" 06c50cee]:
{{{
#!CommitTicketReference repository=""
revision="06c50cee0fb1fd23248518334eabb2fbf61b2cbc"
Fixed #33124 -- Avoided accessing the database connections when not
necessary.