Opened 3 months ago

Closed 2 months ago

Last modified 2 months ago

#35688 closed Bug (fixed)

Postgresql DatabaseWrapper does not allow to override ensure_timezone() function in derived classes

Reported by: Christian Hardenberg Owned by: Sarah Boyce
Component: contrib.postgres Version: 5.1
Severity: Release blocker Keywords: DatabaseWrapper posgresql ensure_timezone
Cc: Christian Hardenberg, Florian Apolloner Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Description:

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

    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:

        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)")

Change History (8)

comment:1 by Sarah Boyce, 3 months ago

Cc: Florian Apolloner added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

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

Would you like to prepare a PR?

comment:2 by Florian Apolloner, 3 months ago

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.

comment:3 by Sarah Boyce, 3 months ago

Easy pickings: unset
Has patch: set
Owner: set to Sarah Boyce
Status: newassigned

I haven't updated ensure_role to be a class method but I can

comment:4 by Natalia Bidart, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:5 by Natalia Bidart, 2 months ago

Triage Stage: Ready for checkinAccepted

comment:6 by Natalia Bidart, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:7 by nessita <124304+nessita@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In 7380ac5:

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+nessita@…>

comment:8 by Natalia <124304+nessita@…>, 2 months ago

In 26c0667:

[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+nessita@…>

Backport of 7380ac57340653854bc2cfe0ed80298cdac6061d from main.

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