Merge pull request #117 from DanCardin/dc/fix-pg11

fix: Compatibility with postgres 11, and allow specifying zero objects.

Author: DanCardin

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## 0.15
 
+### 0.15.12
+- fix: Enable registering zero objects (enable without declaring anything).
+- fix: Add explicit relkind casts to ensure compatibility with postgres 11.
+- fix: Improve quoting of role names, particularly in postgres.
+
 ### 0.15.11
 - fix: Exclude extension-created objects from comparisons.
 - fix: Quote audit column names.

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "sqlalchemy-declarative-extensions"
-version = "0.15.11"
+version = "0.15.12"
 authors = [
     {name = "Dan Cardin", email = "ddcardin@gmail.com"},
 ]

src/sqlalchemy_declarative_extensions/api.py

@@ -224,41 +224,41 @@ def register_create_events(
     from sqlalchemy_declarative_extensions.trigger.ddl import trigger_ddl
     from sqlalchemy_declarative_extensions.view.ddl import view_ddl
 
-    concrete_schemas = metadata.info.get("schemas")
-    concrete_roles = metadata.info.get("roles")
-    concrete_grants = metadata.info.get("grants")
-    concrete_views = Views.extract(metadata)
-    concrete_procedures = metadata.info.get("procedures")
-    concrete_functions = metadata.info.get("functions")
-    concrete_triggers = metadata.info.get("triggers")
-    concrete_databases = metadata.info.get("databases")
-    concrete_rows = metadata.info.get("rows")
-
-    if concrete_databases and databases:
+    concrete_schemas = metadata.info.get("schemas") or Schemas()
+    concrete_roles = metadata.info.get("roles") or Roles()
+    concrete_grants = metadata.info.get("grants") or Grants()
+    concrete_views = Views.extract(metadata) or Views()
+    concrete_procedures = metadata.info.get("procedures") or Procedures()
+    concrete_functions = metadata.info.get("functions") or Functions()
+    concrete_triggers = metadata.info.get("triggers") or Triggers()
+    concrete_databases = metadata.info.get("databases") or Databases()
+    concrete_rows = metadata.info.get("rows") or Rows()
+
+    if databases:
         database_filter = databases if isinstance(databases, list) else None
         event.listen(
             metadata,
             "before_create",
             database_ddl(concrete_databases, database_filter),
         )
 
-    if concrete_schemas and schemas:
+    if schemas:
         schema_filter = schemas if isinstance(schemas, list) else None
         event.listen(
             metadata,
             "before_create",
             schema_ddl(concrete_schemas, schema_filter),
         )
 
-    if concrete_roles and roles:
+    if roles:
         role_filter = roles if isinstance(roles, list) else None
         event.listen(
             metadata,
             "before_create",
             role_ddl(concrete_roles, role_filter),
         )
 
-    if concrete_grants and grants:
+    if grants:
         event.listen(
             metadata,
             "before_create",
@@ -267,39 +267,39 @@ def register_create_events(
         # There should(?) be no need to handle dropping for grants,
         # they will be handled directly by table handling.
 
-    if concrete_views and views:
+    if views:
         view_filter = views if isinstance(views, list) else None
         event.listen(
             metadata,
             "after_create",
             view_ddl(concrete_views, view_filter),
         )
 
-    if concrete_procedures and procedures:
+    if procedures:
         procedure_filter = procedures if isinstance(procedures, list) else None
         event.listen(
             metadata,
             "after_create",
             procedure_ddl(concrete_procedures, procedure_filter),
         )
 
-    if concrete_functions and functions:
+    if functions:
         function_filter = functions if isinstance(functions, list) else None
         event.listen(
             metadata,
             "after_create",
             function_ddl(concrete_functions, function_filter),
         )
 
-    if concrete_triggers and triggers:
+    if triggers:
         trigger_filter = triggers if isinstance(triggers, list) else None
         event.listen(
             metadata,
             "after_create",
             trigger_ddl(concrete_triggers, trigger_filter),
         )
 
-    if concrete_rows and rows:
+    if rows:
         row_filter = rows if isinstance(rows, list) else None
         event.listen(
             metadata,

src/sqlalchemy_declarative_extensions/dialects/postgresql/role.py

@@ -91,29 +91,28 @@ def __repr__(self):
         return f'{cls_name}("{self.name}", {options})'
 
     def to_sql_create(self, raw: bool = True) -> list[str]:
-        segments = ["CREATE ROLE", self.name]
+        segments = ["CREATE ROLE", generic.quote_name(self.name)]
 
         options = postgres_render_role_options(self, raw=raw)
         if options:
             segments.append("WITH")
         segments.extend(options)
 
         if self.in_roles is not None:
-            in_roles = ", ".join(generic.role_names(self.in_roles))
-            segment = f"IN ROLE {in_roles}"
+            segment = f"IN ROLE {generic.quote_names(self.in_roles)}"
             segments.append(segment)
 
         command = " ".join(segments) + ";"
         return [command]
 
     def to_sql_update(self, to_role: Role, raw: bool = True) -> list[str]:
-        role_name = to_role.name
+        role_name = generic.quote_name(to_role.name)
         diff = RoleDiff.diff(self, to_role)
 
         result = []
 
         if self.use_role:
-            result.append(f"SET ROLE {self.use_role};")
+            result.append(f"""SET ROLE "{generic.quote_name(self.use_role)}";""")
 
         diff_options = postgres_render_role_options(diff, raw=raw)
         if diff_options:
@@ -122,10 +121,12 @@ def to_sql_update(self, to_role: Role, raw: bool = True) -> list[str]:
             result.append(alter_role)
 
         for add_name in diff.add_roles:
-            result.append(f"GRANT {add_name} TO {role_name};")
+            result.append(f"""GRANT {generic.quote_name(add_name)} TO {role_name};""")
 
         for remove_name in diff.remove_roles:
-            result.append(f"REVOKE {remove_name} FROM {role_name};")
+            result.append(
+                f"""REVOKE {generic.quote_name(remove_name)} FROM {role_name};"""
+            )
 
         if self.use_role:
             result.append("RESET ROLE")

src/sqlalchemy_declarative_extensions/dialects/postgresql/schema.py

@@ -1,4 +1,7 @@
+from typing import Collection
+
 from sqlalchemy import (
+    BindParameter,
     String,
     and_,
     bindparam,
@@ -17,6 +20,11 @@
 char = CHAR(1)
 
 
+def char_literals(*literals: str) -> Collection[BindParameter]:
+    """Create a collection of CHAR literals."""
+    return [literal(lit, char) for lit in literals]
+
+
 tables = table(
     "tables",
     column("table_schema"),
@@ -209,6 +217,8 @@ def _not_from_extension(obj_id_col, class_name):
     )
 )
 
+object_acl_relkinds = char_literals("r", "S", "f", "n", "T", "v")
+namespace_acl_relkind = literal("n", char)
 object_acl_query = union(
     select(
         pg_namespace.c.nspname.label("schema"),
@@ -222,24 +232,13 @@ def _not_from_extension(obj_id_col, class_name):
             pg_roles, pg_class.c.relowner == pg_roles.c.oid
         )
     )
-    .where(
-        pg_class.c.relkind.cast(char).in_(
-            [
-                literal("r", char),
-                literal("S", char),
-                literal("f", char),
-                literal("n", char),
-                literal("T", char),
-                literal("v", char),
-            ]
-        )
-    )
+    .where(pg_class.c.relkind.cast(char).in_(object_acl_relkinds))
     .where(_table_not_pg)
     .where(_schema_not_pg()),
     select(
         literal(None).label("schema"),
         pg_namespace.c.nspname.label("name"),
-        literal("n").cast(char).label("relkind"),
+        namespace_acl_relkind.label("relkind"),
         pg_roles.c.rolname.label("owner"),
         pg_namespace.c.nspacl.cast(ARRAY(String)),
     )
@@ -257,34 +256,25 @@ def _not_from_extension(obj_id_col, class_name):
     .select_from(
         pg_class.join(pg_namespace, pg_class.c.relnamespace == pg_namespace.c.oid)
     )
-    .where(
-        pg_class.c.relkind.cast(char).in_(
-            [
-                literal("r", char),
-                literal("S", char),
-                literal("f", char),
-                literal("n", char),
-                literal("T", char),
-                literal("v", char),
-            ]
-        )
-    )
+    .where(pg_class.c.relkind.cast(char).in_(object_acl_relkinds))
     .where(_table_not_pg)
     .where(_schema_not_pg())
     .where(_schema_not_from_extension())
 )
 
+view_relkinds = char_literals("v", "m")
+materialized_relkind = literal("m", char)
 views_query = (
     select(
         pg_namespace.c.nspname.label("schema"),
         pg_class.c.relname.label("name"),
         func.pg_get_viewdef(pg_class.c.oid).label("definition"),
-        (pg_class.c.relkind == literal("m")).label("materialized"),
+        (pg_class.c.relkind.cast(char) == materialized_relkind).label("materialized"),
     )
     .select_from(
         pg_class.join(pg_namespace, pg_class.c.relnamespace == pg_namespace.c.oid)
     )
-    .where(pg_class.c.relkind.in_(["v", "m"]))
+    .where(pg_class.c.relkind.cast(char).in_(view_relkinds))
     .where(_schema_not_pg(pg_namespace.c.nspname))
     .where(_schema_not_from_extension())
     .where(_not_from_extension(pg_class.c.oid, "pg_class"))

src/sqlalchemy_declarative_extensions/role/generic.py

@@ -2,6 +2,7 @@
 
 import os
 from dataclasses import dataclass, replace
+from typing import Sequence
 
 from sqlalchemy_declarative_extensions.context import context
 
@@ -78,10 +79,9 @@ def normalize(self):
         )
 
     def to_sql_create(self, raw: bool = True) -> list[str]:
-        statement = f'CREATE ROLE "{self.name}"'
+        statement = f"CREATE ROLE {quote_name(self.name)}"
         if self.in_roles is not None:
-            in_roles = ", ".join(role_names(self.in_roles))
-            statement += f"IN ROLE {in_roles}"
+            statement += f"IN ROLE {quote_names(self.in_roles)}"
         return [statement + ";"]
 
     def to_sql_update(self, to_role, raw: bool = True) -> list[str]:
@@ -90,7 +90,7 @@ def to_sql_update(self, to_role, raw: bool = True) -> list[str]:
         )
 
     def to_sql_drop(self, raw: bool = True) -> list[str]:
-        return [f'DROP ROLE "{self.name}";']
+        return [f"DROP ROLE {quote_name(self.name)};"]
 
     def to_sql_use(self, undo: bool) -> list[str]:
         raise NotImplementedError()
@@ -132,3 +132,11 @@ def role_name(role: Role | str) -> str:
 
 def role_names(roles: list[Role | str]) -> list[str]:
     return [role_name(r) for r in roles]
+
+
+def quote_name(role: Role | str) -> str:
+    return f'"{role_name(role)}"'
+
+
+def quote_names(roles: Sequence[Role | str]) -> str:
+    return ", ".join(quote_name(role) for role in roles)

tests/dialect/postgresql/test_postgres_11.py

@@ -0,0 +1,37 @@
+import pytest
+from pytest_mock_resources import PostgresConfig, create_postgres_fixture
+from sqlalchemy import MetaData, text
+
+from sqlalchemy_declarative_extensions import register_sqlalchemy_events
+from sqlalchemy_declarative_extensions.api import declare_database
+from sqlalchemy_declarative_extensions.role.base import Roles
+
+metadata = MetaData()
+declare_database(metadata, roles=Roles(ignore_unspecified=True))
+
+
+register_sqlalchemy_events(
+    metadata,
+    functions=True,
+    triggers=True,
+    views=True,
+    schemas=True,
+    roles=True,
+    grants=True,
+    procedures=True,
+)
+
+
+@pytest.fixture
+def pmr_postgres_config():
+    return PostgresConfig(port=None, image="postgres:11")
+
+
+pg = create_postgres_fixture(engine_kwargs={"echo": True}, session=True)
+
+
+def test_create(pg):
+    pg.execute(text("CREATE VIEW foo AS SELECT 1"))
+    pg.commit()
+
+    metadata.create_all(bind=pg.connection())

tests/grant/test_only_defined_roles.py

@@ -18,6 +18,7 @@
 class Base(_Base):  # type: ignore
     __abstract__ = True
 
+    roles = ["user"]
     grants = Grants(only_defined_roles=False, ignore_self_grants=False).are(
         DefaultGrant.on_tables_in_schema("public").grant(
             "select",

tests/role/test_remove_valid_until.py

@@ -22,7 +22,7 @@ def test_valid_until_date_being_removed(pg):
         result = compare_roles(conn, roles)
     assert len(result) == 1
     sql = result[0].to_sql()
-    assert sql == ["ALTER ROLE forever_valid WITH VALID UNTIL 'infinity';"]
+    assert sql == ["""ALTER ROLE "forever_valid" WITH VALID UNTIL 'infinity';"""]
 
 
 def test_valid_until_infinity_ignored(pg):

tests/role/test_update_parent_role.py

@@ -41,12 +41,12 @@ def test_createall_role(pg):
 
     child_no_parent_diff = result[0].to_sql()
     assert child_no_parent_diff == [
-        "REVOKE parent1 FROM child_no_parent;",
-        "REVOKE parent2 FROM child_no_parent;",
+        """REVOKE "parent1" FROM "child_no_parent";""",
+        """REVOKE "parent2" FROM "child_no_parent";""",
     ]
 
     child_with_parent_diff = result[1].to_sql()
     assert child_with_parent_diff == [
-        "GRANT parent1 TO child_with_parent;",
-        "GRANT parent2 TO child_with_parent;",
+        """GRANT "parent1" TO "child_with_parent";""",
+        """GRANT "parent2" TO "child_with_parent";""",
     ]