Copilot commented on code in PR #42312:
URL: https://github.com/apache/superset/pull/42312#discussion_r3640158722
##########
superset/db_engine_specs/postgres.py:
##########
@@ -793,6 +793,28 @@ def get_catalog_names(
)
}
+ @classmethod
+ def get_schema_names(cls, inspector: Inspector) -> set[str]:
+ """
+ Return all schema names, excluding actual Postgres system schemas.
+
+ SQLAlchemy's Postgres dialect filters out system schemas with the
+ query ``nspname NOT LIKE 'pg_%'``. Since ``_`` is a single-character
+ wildcard in SQL ``LIKE`` patterns, this unintentionally excludes any
+ user-defined schema that merely starts with ``pg`` followed by any
+ other character (e.g. ``pgsql``, ``pgstats``), not only the actual
+ Postgres system schemas, which are always prefixed with the literal
+ string ``pg_`` (e.g. ``pg_catalog``, ``pg_toast``).
+ """
Review Comment:
The docstring says this method excludes "actual Postgres system schemas" and
that those schemas "always" start with the literal `pg_`, but PostgreSQL also
exposes non-`pg_` system schemas like `information_schema` (which this method
intentionally keeps). Consider tightening the wording to explicitly describe
the `pg_`-prefix exclusion so the doc stays accurate.
This issue also appears on line 809 of the same file.
##########
tests/unit_tests/db_engine_specs/test_postgres.py:
##########
@@ -403,3 +403,33 @@ def test_interval_type_mutator() -> None:
assert mutator(True) is None
assert mutator([1, 2, 3]) is None
assert mutator({"days": 1}) is None
+
+
+def test_get_schema_names_excludes_only_actual_system_schemas(
+ mocker: MockerFixture,
+) -> None:
+ """
+ DB Eng Specs (postgres): Test ``get_schema_names``
+
+ User-defined schemas that merely start with ``pg`` (but are not
+ actual Postgres system schemas, which always start with the literal
+ ``pg_``) must not be filtered out. See SIP/issue #30678.
+ """
Review Comment:
The test docstring references "SIP/issue #30678", but #30678 is a GitHub
issue (not a SIP). Updating the wording avoids confusion when someone searches
for the referenced artifact.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
