Fix quotation marks being included in search path

[libcthorne]

[PostgreSQL version: 11.1]

Schema names containing underscores are wrapped in quotation marks by postgres when running SHOW search_path:

db=# SET search_path = 'country_BD';
SET

db=# SHOW search_path;
 search_path
--------------
 "country_BD"
(1 row)

Without the underscore, the quotation marks are not present:

db=# SET search_path = 'country';
SET

db=# SHOW search_path;
 search_path
-------------
 country
(1 row)

As a result, when a schema name contains an underscore, the search path restoration logic ends up adding quotes each time and table lookups stop working:

db=# SET search_path = 'country_A';
SET
db=# SHOW search_path;
 search_path
-------------
 "country_A"
(1 row)

db=# SET search_path = '"country_A"';
SET
db=# SHOW search_path;
   search_path
-----------------
 """country_A"""
(1 row)

This PR extends #693 to fix the issue.

[codecov-commenter]

Codecov Report

Base: 87.21% // Head: 87.21% // No change to project coverage 👍

Coverage data is based on head (c537099) compared to base (e84624f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #829   +/-   ##
=======================================
  Coverage   87.21%   87.21%           
=======================================
  Files          52       52           
  Lines        2363     2363           
=======================================
  Hits         2061     2061           
  Misses        302      302           

Impacted Files	Coverage Δ
django_tenants/postgresql_backend/introspection.py	72.41% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tomturner]

@atodorov are you happy with this?

[atodorov]

@atodorov are you happy with this?

The change itself looks good but given the importance it needs a test scenario. I can look into testing over the weekend if @libcthorne doesn't beat me to it first.

[libcthorne]

@atodorov are you happy with this?

The change itself looks good but given the importance it needs a test scenario. I can look into testing over the weekend if @libcthorne doesn't beat me to it first.

Feel free to take this on, otherwise I can look in the coming weeks!

[atodorov]

[PostgreSQL version: 11.1]

During my tests in #921 I've discovered that later versions also add the double quotes. For example:

2023-04-19T16:00:42.9978058Z ***** DEBUG: raw data from Postgres: ('"tenant`1", public',)
2023-04-19T16:00:43.0302627Z ***** DEBUG: raw data from Postgres: ('"tenant-2", public',)

Schema names containing underscores are wrapped in quotation marks by postgres when running

FTR that seems true for country_BD, maybe b/c it contains upper-case letters, but didn't hold true for schema names such as tenant_1 and tenant_2 that I tried with.

Regardless of the facts above I can't reproduce a failure in #921, even without this patch.

If the logs are inspected we can see that there are search paths which are quoted with double quotes, raw data from Postgres. These are usually schema names containing special characters.

There's also an explicit test called test_switching_search_path which switches between 2 tenants and asserts on the number of objects inside of them (different numbers).

@libcthorne can you provide some more information about the reproducer?

At the time when you opened this PR did you use a version of django-tenants with #693 applied, (v3.4.0 or later)?

Which Python version did you have installed ?

[libcthorne]

At the time when you opened this PR did you use a version of django-tenants with #693 applied, (v3.4.0 or later)?

Which Python version did you have installed ?

This was using django-tenants 3.4.5 and Python 3.10. The issue only appeared after PR #693.

[atodorov]

This was using django-tenants 3.4.5 and Python 3.10. The issue only appeared after PR #693.

Unfortunately I'm failing to update the test suite in a way that would use version 3.4.5 to verify the reproducer. It has to do with how we setup the environment (python setup.py develop).

@libcthorne does the failure happen for you with the latest release of django-tenants ? I'm trying to figure out if there's more to the reproducer than meets the eye.

@tomturner I'm on the fence about this PR. On one hand it looks sane and obviously solves a problem. On the other hand unmodified django-tenants seems to work fine and not be able to trigger the original issue so why bother changing. WDYT ?

[libcthorne]

I confirm the issue is still present on the latest django-tenants (3.4.8) and and Python 3.11.

The context is that when using pytest with --create-db, the database setup fails as it tries to create the same table twice. At some point there's a table lookup to see if the table already exists before attempting creation but it gets it wrong due to the incorrect search path.

../.cache/pypoetry/virtualenvs/wfp-shf-O1g-rtR2-py3.11/lib/python3.11/site-packages/django_tenants/management/commands/migrate_schemas.py:77: in handle
    executor.run_migrations(tenants=tenants)
../.cache/pypoetry/virtualenvs/wfp-shf-O1g-rtR2-py3.11/lib/python3.11/site-packages/django_tenants/migration_executors/standard.py:14: in run_migrations
    run_migrations(self.args, self.options, self.codename, schema_name, idx=idx, count=len(tenants))
../.cache/pypoetry/virtualenvs/wfp-shf-O1g-rtR2-py3.11/lib/python3.11/site-packages/django_tenants/migration_executors/base.py:48: in run_migrations
    migration_recorder.ensure_schema()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.migrations.recorder.MigrationRecorder object at 0x7f52dac78d10>

    def ensure_schema(self):
        """Ensure the table exists and has the correct schema."""
        # If the table's there, that's fine - we've never changed its schema
        # in the codebase.
        if self.has_table():
            return
        # Make the table
        try:
            with self.connection.schema_editor() as editor:
                editor.create_model(self.Migration)
        except DatabaseError as exc:
>           raise MigrationSchemaMissing(
                "Unable to create the django_migrations table (%s)" % exc
            )
E           django.db.migrations.exceptions.MigrationSchemaMissing: Unable to create the django_migrations table (relation "django_migrations" already exists
E           )

I am working around it by manually applying the patch in this PR using a custom backend:

DATABASES = {
    "default": {
        "ENGINE": "wfp_shf.apps.multitenancy.postgresql_backend",
        ...

class DatabaseWrapper(DjangoTenantsDatabaseWrapper):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.introspection = DatabaseSchemaIntrospection(self)

from django.db.backends.postgresql.introspection import DatabaseIntrospection


class DatabaseSchemaIntrospectionSearchPathContext:
    """
    This context manager restores the original search path of the cursor
    once the method of the introspection class has been called.
    """

    def __init__(self, cursor, connection):
        self.cursor = cursor
        self.connection = connection
        self.original_search_path = None

    def __enter__(self):
        self.cursor.execute("SHOW search_path")
        self.original_search_path = [
            search_path.strip().replace('"', "")
            for search_path in self.cursor.fetchone()[0].split(",")
        ]
        self.cursor.execute(f"SET search_path = '{self.connection.schema_name}'")

    def __exit__(self, *args, **kwargs):
        formatted_search_paths = ", ".join(
            f"'{search_path}'" for search_path in self.original_search_path
        )
        self.cursor.execute(f"SET search_path = {formatted_search_paths}")


class DatabaseSchemaIntrospection(DatabaseIntrospection):
    """
    database schema introspection class
    """

    def get_table_list(self, cursor):
        with DatabaseSchemaIntrospectionSearchPathContext(
            cursor=cursor, connection=self.connection
        ):
            return super().get_table_list(cursor)

    def get_table_description(self, cursor, table_name):
        with DatabaseSchemaIntrospectionSearchPathContext(
            cursor=cursor, connection=self.connection
        ):
            return super().get_table_description(cursor, table_name)

    def get_sequences(self, cursor, table_name, table_fields=()):
        with DatabaseSchemaIntrospectionSearchPathContext(
            cursor=cursor, connection=self.connection
        ):
            return super().get_sequences(cursor, table_name, table_fields)

    def get_key_columns(self, cursor, table_name):
        with DatabaseSchemaIntrospectionSearchPathContext(
            cursor=cursor, connection=self.connection
        ):
            return super().get_key_columns(cursor, table_name)

    def get_constraints(self, cursor, table_name):
        with DatabaseSchemaIntrospectionSearchPathContext(
            cursor=cursor, connection=self.connection
        ):
            return super().get_constraints(cursor, table_name)