Skip to content

Commit e1bfcc1

Browse files
fix(drop): drop a PostgreSQL enum type only when no column uses it
On PostgreSQL, dropping a table could drop columns of other tables, with their data. An enum attribute is declared with an enum type named by its values, so every column in the schema with the same values shares one type. `drop_quick()` dropped the table's enum types afterwards with `DROP TYPE ... CASCADE`, and CASCADE also dropped every other column of that type. Two tables that both declare `species : enum('mouse', 'rat')` share a type, and dropping one took the other's `species` column. Every jobs table has the same `status` enum, so dropping one jobs table removed `status` from all the others in the schema. MySQL has no enum types and was not affected. An enum type is now dropped only when no column uses it any more, and the statement no longer cascades, so a type still in use is never dropped with its columns. The new tests, on both backends, drop one of two tables that share an enum and check that the other keeps its column and values, that the type is removed once neither table remains, and that dropping one jobs table leaves another's status column and jobs in place. Both fail on PostgreSQL before this change. (cherry picked from commit 7e9cae9)
1 parent 68fdea5 commit e1bfcc1

4 files changed

Lines changed: 173 additions & 10 deletions

File tree

‎src/datajoint/adapters/postgres.py‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,10 +1591,21 @@ def drop_enum_types_for_table(self, schema_name: str, table_name: str) -> list[s
15911591
# to get actual enum types, then call this with results
15921592
return [] # Placeholder - actual implementation requires query execution
15931593

1594+
def _quote_enum_type_name(self, enum_type_name: str) -> str:
1595+
"""Quote a ``schema.typename`` enum type name, each part separately."""
1596+
parts = enum_type_name.split(".")
1597+
if len(parts) == 2:
1598+
return f"{self.quote_identifier(parts[0])}.{self.quote_identifier(parts[1])}"
1599+
return self.quote_identifier(enum_type_name)
1600+
15941601
def drop_enum_type_ddl(self, enum_type_name: str) -> str:
15951602
"""
15961603
Generate DROP TYPE IF EXISTS statement for a PostgreSQL enum.
15971604
1605+
The statement has no CASCADE: enum types are named by their values and
1606+
shared by every column in the schema with the same values, and CASCADE
1607+
would drop those columns too. A type still in use is not dropped.
1608+
15981609
Parameters
15991610
----------
16001611
enum_type_name : str
@@ -1603,12 +1614,26 @@ def drop_enum_type_ddl(self, enum_type_name: str) -> str:
16031614
Returns
16041615
-------
16051616
str
1606-
DROP TYPE IF EXISTS statement with CASCADE.
1617+
DROP TYPE IF EXISTS statement.
16071618
"""
1608-
# Split schema.typename and quote each part
1609-
parts = enum_type_name.split(".")
1610-
if len(parts) == 2:
1611-
qualified_name = f"{self.quote_identifier(parts[0])}.{self.quote_identifier(parts[1])}"
1612-
else:
1613-
qualified_name = self.quote_identifier(enum_type_name)
1614-
return f"DROP TYPE IF EXISTS {qualified_name} CASCADE"
1619+
return f"DROP TYPE IF EXISTS {self._quote_enum_type_name(enum_type_name)}"
1620+
1621+
def enum_type_in_use_sql(self, enum_type_name: str) -> str:
1622+
"""
1623+
Query that returns a row if any table column uses a PostgreSQL enum type.
1624+
1625+
Parameters
1626+
----------
1627+
enum_type_name : str
1628+
Fully qualified enum type name (schema.typename).
1629+
1630+
Returns
1631+
-------
1632+
str
1633+
SQL query returning at most one row.
1634+
"""
1635+
return (
1636+
"SELECT 1 FROM pg_catalog.pg_attribute "
1637+
f"WHERE atttypid = {self.quote_string(self._quote_enum_type_name(enum_type_name))}::regtype "
1638+
"AND attnum > 0 AND NOT attisdropped LIMIT 1"
1639+
)

‎src/datajoint/table.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,15 +1247,20 @@ def drop_quick(self):
12471247
self.connection.query(query)
12481248
logger.info("Dropped table %s" % self.full_table_name)
12491249

1250-
# For PostgreSQL, clean up enum types after dropping the table
1250+
# For PostgreSQL, clean up enum types after dropping the table. An enum
1251+
# type is named by its values and shared by every column in the schema
1252+
# with the same values (every jobs table's status, for one), so it is
1253+
# dropped only when no column uses it any more.
12511254
if enum_types_to_drop and hasattr(adapter, "drop_enum_type_ddl"):
12521255
for enum_type in enum_types_to_drop:
12531256
try:
1257+
if self.connection.query(adapter.enum_type_in_use_sql(enum_type)).fetchone():
1258+
continue
12541259
drop_ddl = adapter.drop_enum_type_ddl(enum_type)
12551260
self.connection.query(drop_ddl)
12561261
logger.debug("Dropped enum type %s" % enum_type)
12571262
except Exception:
1258-
pass # Ignore errors - type may be used by other tables
1263+
pass # Ignore errors - enum cleanup is best-effort
12591264
else:
12601265
logger.info("Nothing to drop: table %s is not declared" % self.full_table_name)
12611266

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
"""
2+
Dropping a table must not drop columns of other tables.
3+
4+
On PostgreSQL, an enum attribute is declared with an enum type named by its
5+
values, so every column in the schema with the same values shares one type.
6+
Dropping a table drops its enum types only when no other column uses them.
7+
"""
8+
9+
import time
10+
11+
import pytest
12+
13+
import datajoint as dj
14+
15+
16+
@pytest.fixture(scope="function")
17+
def schema_by_backend(connection_by_backend, db_creds_by_backend):
18+
backend = db_creds_by_backend["backend"]
19+
schema_name = f"djtest_shared_enum_{backend}_{str(int(time.time() * 1000))[-8:]}"
20+
schema = dj.Schema(schema_name, connection=connection_by_backend)
21+
yield schema
22+
if connection_by_backend.is_connected:
23+
connection_by_backend.query(
24+
f"DROP DATABASE IF EXISTS {connection_by_backend.adapter.quote_identifier(schema_name)}"
25+
if backend == "mysql"
26+
else f"DROP SCHEMA IF EXISTS {connection_by_backend.adapter.quote_identifier(schema_name)} CASCADE"
27+
)
28+
29+
30+
def _columns(schema, table_name):
31+
rows = schema.connection.query(
32+
"SELECT column_name FROM information_schema.columns "
33+
"WHERE table_schema = %s AND table_name = %s ORDER BY ordinal_position",
34+
args=(schema.database, table_name),
35+
).fetchall()
36+
return [row[0] for row in rows]
37+
38+
39+
def _enum_types(schema):
40+
if schema.connection.adapter.backend != "postgresql":
41+
return []
42+
rows = schema.connection.query(
43+
"SELECT t.typname FROM pg_catalog.pg_type t "
44+
"JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace "
45+
"WHERE t.typtype = 'e' AND n.nspname = %s",
46+
args=(schema.database,),
47+
).fetchall()
48+
return [row[0] for row in rows]
49+
50+
51+
def test_drop_keeps_a_shared_enum_column(schema_by_backend):
52+
"""Two tables declare the same enum. Dropping one keeps the other's column
53+
and its values; dropping the second removes the type."""
54+
55+
@schema_by_backend
56+
class Animal(dj.Manual):
57+
definition = """
58+
animal_id : int32
59+
---
60+
species : enum('mouse', 'rat')
61+
"""
62+
63+
@schema_by_backend
64+
class Cage(dj.Manual):
65+
definition = """
66+
cage_id : int32
67+
---
68+
species : enum('mouse', 'rat')
69+
"""
70+
71+
Animal.insert([(1, "mouse"), (2, "rat")])
72+
Cage.insert([(1, "rat")])
73+
types_before = _enum_types(schema_by_backend)
74+
75+
Cage().drop_quick()
76+
assert _columns(schema_by_backend, "animal") == ["animal_id", "species"]
77+
assert Animal.to_dicts(order_by="animal_id") == [
78+
{"animal_id": 1, "species": "mouse"},
79+
{"animal_id": 2, "species": "rat"},
80+
]
81+
assert _enum_types(schema_by_backend) == types_before
82+
83+
Animal().drop_quick()
84+
assert _enum_types(schema_by_backend) == []
85+
86+
87+
def test_dropping_a_jobs_table_keeps_other_jobs_tables_whole(schema_by_backend):
88+
"""Every jobs table's status column has the same enum. Dropping one jobs
89+
table leaves the others' status columns, and their jobs, in place."""
90+
91+
@schema_by_backend
92+
class Item(dj.Manual):
93+
definition = """
94+
item : int32
95+
"""
96+
97+
@schema_by_backend
98+
class First(dj.Computed):
99+
definition = """
100+
-> Item
101+
"""
102+
103+
def make(self, key):
104+
self.insert1(key)
105+
106+
@schema_by_backend
107+
class Second(dj.Computed):
108+
definition = """
109+
-> Item
110+
"""
111+
112+
def make(self, key):
113+
raise RuntimeError("not ready")
114+
115+
Item.insert([(1,), (2,)])
116+
First.populate(reserve_jobs=True)
117+
Second.populate(reserve_jobs=True, suppress_errors=True)
118+
119+
First.jobs.drop_quick()
120+
assert "status" in _columns(schema_by_backend, "~~second")
121+
assert sorted(Second.jobs.to_arrays("status").tolist()) == ["error", "error"]

‎tests/unit/test_adapters.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,18 @@ def test_enum_type_sql(self, adapter):
352352
assert "'pending'" in sql
353353
assert "'complete'" in sql
354354

355+
def test_drop_enum_type_ddl_does_not_cascade(self, adapter):
356+
"""An enum type shared by other columns must not take them with it."""
357+
sql = adapter.drop_enum_type_ddl("myschema.enum_abc")
358+
assert sql == 'DROP TYPE IF EXISTS "myschema"."enum_abc"'
359+
360+
def test_enum_type_in_use_sql(self, adapter):
361+
"""The usage check looks the type up by its quoted, qualified name."""
362+
sql = adapter.enum_type_in_use_sql("myschema.enum_abc")
363+
assert "pg_attribute" in sql
364+
assert """'"myschema"."enum_abc"'::regtype""" in sql
365+
assert "NOT attisdropped" in sql
366+
355367
def test_drop_enum_type_sql(self, adapter):
356368
"""Test PostgreSQL enum type dropping."""
357369
sql = adapter.drop_enum_type_sql("myschema", "mytable", "status")

0 commit comments

Comments
 (0)