fix: role name quoting.

Author: DanCardin

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### 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.

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/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/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";""",
     ]