Skip to content

Commit f07cc33

Browse files
committed
User must be OWNER of referring tables
Prior to this change, there was no check to verify that the user was an OWNER of the referring tables (tables that have fks pointing to the table to be repacked). This is a problem because Psycopack needs to create new foreign keys on that table that point to the shadow table, and that is not possible unless the user is the table owner. This change adds an explicit check to ensure the user is the owner of the referring tables before proceeding.
1 parent c332b2b commit f07cc33

4 files changed

Lines changed: 72 additions & 3 deletions

File tree

src/psycopack/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
InvalidPrimaryKeyTypeForConversion,
1212
InvalidStageForReset,
1313
NoCreateAndUsagePrivilegeOnSchema,
14+
NoReferringTableOwnership,
1415
NotTableOwner,
1516
PrimaryKeyNotFound,
1617
ReferringForeignKeyInDifferentSchema,
@@ -33,6 +34,7 @@
3334
"InvalidPrimaryKeyTypeForConversion",
3435
"InvalidStageForReset",
3536
"NoCreateAndUsagePrivilegeOnSchema",
37+
"NoReferringTableOwnership",
3638
"NotTableOwner",
3739
"PrimaryKeyNotFound",
3840
"ReferringForeignKeyInDifferentSchema",

src/psycopack/_introspect.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class ReferringForeignKey:
3030
is_validated: bool
3131
referring_table: str
3232
schema: str
33+
is_owned_by_user: bool
3334

3435

3536
@dataclasses.dataclass
@@ -239,7 +240,16 @@ def get_referring_fks(self, *, table: str) -> list[ReferringForeignKey]:
239240
pg_get_constraintdef(pg_constraint.oid) AS definition,
240241
pg_constraint.convalidated AS is_validated,
241242
referring_pg_class.relname AS referring_table,
242-
pg_namespace.nspname AS schema
243+
pg_namespace.nspname AS schema,
244+
(
245+
SELECT
246+
tableowner = current_user
247+
FROM
248+
pg_tables
249+
WHERE
250+
schemaname = {schema}
251+
AND tablename = referring_pg_class.relname
252+
) AS is_owned_by_user
243253
FROM
244254
pg_catalog.pg_constraint
245255
INNER JOIN
@@ -260,7 +270,10 @@ def get_referring_fks(self, *, table: str) -> list[ReferringForeignKey]:
260270
definition;
261271
""")
262272
)
263-
.format(table=psycopg.sql.Literal(table))
273+
.format(
274+
table=psycopg.sql.Literal(table),
275+
schema=psycopg.sql.Literal(self.schema),
276+
)
264277
.as_string(self.conn)
265278
)
266279
results = self.cur.fetchall()
@@ -272,8 +285,9 @@ def get_referring_fks(self, *, table: str) -> list[ReferringForeignKey]:
272285
is_validated=is_validated,
273286
referring_table=referring_table,
274287
schema=schema,
288+
is_owned_by_user=is_owned_by_user,
275289
)
276-
for name, definition, is_validated, referring_table, schema in results
290+
for name, definition, is_validated, referring_table, schema, is_owned_by_user in results
277291
]
278292

279293
def table_is_empty(self, *, table: str) -> int:

src/psycopack/_repack.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ class NotTableOwner(BaseRepackError):
7272
pass
7373

7474

75+
class NoReferringTableOwnership(BaseRepackError):
76+
pass
77+
78+
7579
class PostBackfillBatchCallback(typing.Protocol):
7680
def __call__(
7781
self, batch: _introspect.BackfillBatch, /
@@ -804,3 +808,19 @@ def _check_user_permissions(self) -> None:
804808
f"your user via:\n"
805809
f"ALTER TABLE {self.schema}.{self.table} OWNER TO {user};"
806810
)
811+
812+
referring_tables_without_ownership = [
813+
f"{fk.schema}.{fk.referring_table}"
814+
for fk in self.introspector.get_referring_fks(table=self.table)
815+
if not fk.is_owned_by_user
816+
]
817+
if referring_tables_without_ownership:
818+
user = self.introspector.get_user()
819+
message = (
820+
f"Psycopack requires the user to have ownership of tables that "
821+
f"have foreign keys pointing to the table {self.schema}.{self.table}. "
822+
f"You can grant ownership to your user via the following commands:\n"
823+
)
824+
for table in referring_tables_without_ownership:
825+
message += f"ALTER TABLE {table} OWNER TO {user};\n"
826+
raise NoReferringTableOwnership(message)

tests/test_repack.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
InvalidPrimaryKeyTypeForConversion,
1414
InvalidStageForReset,
1515
NoCreateAndUsagePrivilegeOnSchema,
16+
NoReferringTableOwnership,
1617
NotTableOwner,
1718
PrimaryKeyNotFound,
1819
ReferringForeignKeyInDifferentSchema,
@@ -1752,3 +1753,35 @@ def test_user_without_table_ownership(
17521753
cur=cur,
17531754
schema=schema,
17541755
)
1756+
1757+
1758+
def test_user_without_referring_table_ownership(
1759+
connection: _psycopg.Connection,
1760+
) -> None:
1761+
schema = "sweet_schema"
1762+
with connection.cursor() as cur:
1763+
cur.execute(f"CREATE SCHEMA {schema};")
1764+
cur.execute(f"REVOKE CREATE ON SCHEMA {schema} FROM PUBLIC;")
1765+
factories.create_table_for_repacking(
1766+
connection=connection,
1767+
cur=cur,
1768+
table_name="to_repack",
1769+
rows=10,
1770+
schema=schema,
1771+
)
1772+
cur.execute("DROP USER IF EXISTS sweet_user;")
1773+
cur.execute("CREATE USER sweet_user;")
1774+
cur.execute("GRANT CREATE, USAGE ON SCHEMA sweet_schema TO sweet_user;")
1775+
cur.execute("ALTER TABLE sweet_schema.to_repack OWNER TO sweet_user;")
1776+
cur.execute("SET ROLE sweet_user;")
1777+
with pytest.raises(
1778+
NoReferringTableOwnership,
1779+
match="ALTER TABLE sweet_schema.referring_table OWNER TO sweet_user;",
1780+
):
1781+
Repack(
1782+
table="to_repack",
1783+
batch_size=1,
1784+
conn=connection,
1785+
cur=cur,
1786+
schema=schema,
1787+
)

0 commit comments

Comments
 (0)