[Django] #35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone() function in derived classes

12 views
Skip to first unread message

Django

unread,
Aug 18, 2024, 3:07:19 AM8/18/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: chardenberg | Type: Bug
Status: new | Component:
| contrib.postgres
Version: 5.1 | Severity: Normal
Keywords: DatabaseWrapper | Triage Stage:
posgresql ensure_timezone | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
**Description:**

The function _configure_connection() in base.py of the postgresql
DatabaseWrapper directly calls the local function ensure_timezone:

{{{#!python
def _configure_connection(self, connection):
# This function is called from init_connection_state and from the
# psycopg pool itself after a connection is opened. Make sure that
# whatever is done here does not access anything on self aside
from
# variables.

# Commit after setting the time zone.
commit_tz = ensure_timezone(connection, self.ops,
self.timezone_name)
# Set the role on the connection. This is useful if the credential
used
# to login is not the same as the role that owns database
resources. As
# can be the case when using temporary or ephemeral credentials.
role_name = self.settings_dict["OPTIONS"].get("assume_role")
commit_role = ensure_role(connection, self.ops, role_name)

return commit_role or commit_tz
}}}

Instead it should call the class method like this, which should have the
same result, but allows overriding in derived classes:
{{{#!python
commit_tz = self.ensure_timezone()
}}}

**Why it matters?**
I am implementing a database wrapper for QuestDB, which implements the
postgres-protocol, so it's useful to derive the wrapper from the postgres
wrapper. I need to override some methods though, such as ensure_timezone,
because the set_config function is not supported by QuestDB ("SELECT
set_config('TimeZone', 'UTC', false)")
--
Ticket URL: <https://code.djangoproject.com/ticket/35688>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 19, 2024, 3:03:43 AM8/19/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: (none)
Hardenberg |
Type: Bug | Status: new
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: DatabaseWrapper | Triage Stage: Accepted
posgresql ensure_timezone |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* cc: Florian Apolloner (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted

Comment:

Thank you! Agree we should make some kinda update here
Regression in fad334e1a9b54ea1acb8cce02a25934c5acfe99f (ref #33497)

Would you like to prepare a PR?
--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:1>

Django

unread,
Aug 19, 2024, 3:55:11 AM8/19/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: (none)
Hardenberg |
Type: Bug | Status: new
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: DatabaseWrapper | Triage Stage: Accepted
posgresql ensure_timezone |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Florian Apolloner):

Yes we can make it a class method (and init_connection_state +
ensure_role) as well. Please ping me for a review once a PR is up.
--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:2>

Django

unread,
Aug 19, 2024, 10:54:52 AM8/19/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: Sarah
Hardenberg | Boyce
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: DatabaseWrapper | Triage Stage: Accepted
posgresql ensure_timezone |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* easy: 1 => 0
* has_patch: 0 => 1
* owner: (none) => Sarah Boyce
* status: new => assigned

Comment:

I haven't updated ensure_role to be a class method but I can
--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:3>

Django

unread,
Aug 23, 2024, 10:21:06 AM8/23/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: Sarah
Hardenberg | Boyce
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: DatabaseWrapper | Triage Stage: Ready for
posgresql ensure_timezone | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

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

Django

unread,
Aug 27, 2024, 10:52:56 AM8/27/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: Sarah
Hardenberg | Boyce
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: DatabaseWrapper | Triage Stage: Accepted
posgresql ensure_timezone |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:5>

Django

unread,
Aug 28, 2024, 6:22:28 PM8/28/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: Sarah
Hardenberg | Boyce
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: DatabaseWrapper | Triage Stage: Ready for
posgresql ensure_timezone | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:6>

Django

unread,
Aug 28, 2024, 6:25:16 PM8/28/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: Sarah
Hardenberg | Boyce
Type: Bug | Status: closed
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution: fixed
Keywords: DatabaseWrapper | Triage Stage: Ready for
posgresql ensure_timezone | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by nessita <124304+nessita@…>):

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

Comment:

In [changeset:"7380ac57340653854bc2cfe0ed80298cdac6061d" 7380ac5]:
{{{#!CommitTicketReference repository=""
revision="7380ac57340653854bc2cfe0ed80298cdac6061d"
Fixed #35688 -- Restored timezone and role setters to be PostgreSQL
DatabaseWrapper methods.

Following the addition of PostgreSQL connection pool support in
Refs #33497, the methods for configuring the database role and timezone
were moved to module-level functions. This change prevented subclasses
of DatabaseWrapper from overriding these methods as needed, for example,
when creating wrappers for other PostgreSQL-based backends.

Thank you Christian Hardenberg for the report and to
Florian Apolloner and Natalia Bidart for the review.

Regression in fad334e1a9b54ea1acb8cce02a25934c5acfe99f.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:7>

Django

unread,
Aug 28, 2024, 6:26:44 PM8/28/24
to django-...@googlegroups.com
#35688: Postgresql DatabaseWrapper does not allow to override ensure_timezone()
function in derived classes
-------------------------------------+-------------------------------------
Reporter: Christian | Owner: Sarah
Hardenberg | Boyce
Type: Bug | Status: closed
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution: fixed
Keywords: DatabaseWrapper | Triage Stage: Ready for
posgresql ensure_timezone | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):

In [changeset:"26c06671d9f9ff679be5e290b0752a45e582ce0c" 26c0667]:
{{{#!CommitTicketReference repository=""
revision="26c06671d9f9ff679be5e290b0752a45e582ce0c"
[5.1.x] Fixed #35688 -- Restored timezone and role setters to be
PostgreSQL DatabaseWrapper methods.

Following the addition of PostgreSQL connection pool support in
Refs #33497, the methods for configuring the database role and timezone
were moved to module-level functions. This change prevented subclasses
of DatabaseWrapper from overriding these methods as needed, for example,
when creating wrappers for other PostgreSQL-based backends.

Thank you Christian Hardenberg for the report and to
Florian Apolloner and Natalia Bidart for the review.

Regression in fad334e1a9b54ea1acb8cce02a25934c5acfe99f.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>

Backport of 7380ac57340653854bc2cfe0ed80298cdac6061d from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35688#comment:8>
Reply all
Reply to author
Forward
0 new messages