Preserve attribute type metadata across Table.alter

An attribute added by alter() never had its declared type recorded on
backends that store column comments out of line. The comment carries
the `:type:` prefix that heading reads back as original_type, so on
PostgreSQL the column came back as its generated type name: describe()
no longer round-tripped, and since describe() feeds the next alter(),
the table became permanently un-alterable.

declare.alter() now returns the new definition's column comments and
Table.alter reapplies them after the ALTER, mirroring what declare()
already does with its post-DDL.

Emit CREATE TYPE only for types the new definition adds, by
subtracting what the old definition registered rather than discarding
it. Previously every alter of a table with any pre-existing enum
issued a statement certain to fail, which the surrounding guard then
swallowed, leaving no way to tell an expected collision from a real
error. Enum type names are content hashes shared across a schema, so
a collision remains possible and the guard stays -- but it now logs
instead of discarding.

Give the base adapter a get_pending_enum_ddl returning nothing, so the
call site can drop its hasattr check and match the capability
properties alongside it.

Assert the added column's original_type and alter a second time, which
is what fails when the type is not recoverable.

Author: MilagrosMarin

src/datajoint/adapters/base.py

@@ -617,6 +617,26 @@ def supports_inline_indexes(self) -> bool:
         """
         return True  # Default for MySQL, override in PostgreSQL
 
+    def get_pending_enum_ddl(self, schema_name: str) -> list[str]:
+        """
+        DDL for backend types that must exist before the columns using them,
+        clearing the pending list as it reads.
+
+        Backends that spell column types inline (MySQL) have none. PostgreSQL
+        overrides this to emit CREATE TYPE for enums registered while parsing.
+
+        Parameters
+        ----------
+        schema_name : str
+            Schema used to qualify the type names.
+
+        Returns
+        -------
+        list[str]
+            Empty for MySQL; CREATE TYPE statements for PostgreSQL.
+        """
+        return []
+
     @property
     def supports_column_position(self) -> bool:
         """

src/datajoint/declare.py

@@ -678,7 +678,7 @@ def alter(
     adapter,
     *,
     schema_name: str | None = None,
-) -> tuple[list[str], list[str], list[str]]:
+) -> tuple[list[str], list[str], list[str], dict]:
     """
     Generate SQL ALTER commands for table definition changes.
 
@@ -700,11 +700,15 @@ def alter(
     Returns
     -------
     tuple
-        Three-element tuple:
+        Four-element tuple:
 
         - sql : list[str] - SQL ALTER commands
         - new_stores : list[str] - New external stores used
         - pre_ddl : list[str] - DDL to run before the ALTER (e.g. CREATE TYPE)
+        - column_comments : dict - Comments to reapply after the ALTER. On
+          backends that store them out of line these carry the ``:type:``
+          prefix that ``heading`` reads back as ``original_type``, so skipping
+          them silently loses the declared type of an added attribute.
 
     Raises
     ------
@@ -719,16 +723,13 @@ def alter(
         index_sql,
         external_stores,
         _fk_attribute_map,
-        _column_comments,
+        column_comments,
     ) = prepare_declare(definition, context, adapter)
 
-    # prepare_declare registers backend types (PostgreSQL enums) on the adapter as
-    # a side effect. Drain them here, between the two parses: the old definition's
-    # types already exist in the database, so draining after both would emit
-    # CREATE TYPE for those as well.
-    pre_ddl = []
-    if schema_name and hasattr(adapter, "get_pending_enum_ddl"):
-        pre_ddl.extend(adapter.get_pending_enum_ddl(schema_name))
+    # prepare_declare registers backend types (PostgreSQL enums) on the adapter
+    # as a side effect, so each parse must be drained separately to tell the two
+    # apart. Type names are content hashes, making the statements comparable.
+    new_type_ddl = adapter.get_pending_enum_ddl(schema_name) if schema_name else []
 
     (
         table_comment_,
@@ -741,10 +742,12 @@ def alter(
         _column_comments_,
     ) = prepare_declare(old_definition, context, adapter)
 
-    # Discard what the old definition registered, so it cannot leak into the next
-    # declare() on this adapter (get_pending_enum_ddl clears as it reads).
-    if schema_name and hasattr(adapter, "get_pending_enum_ddl"):
-        adapter.get_pending_enum_ddl(schema_name)
+    # Whatever the old definition registered already exists in the database, so
+    # only the difference needs creating. Draining both also leaves nothing
+    # behind to leak into the next declare() on this adapter, including on the
+    # NotImplementedError paths below.
+    old_type_ddl = set(adapter.get_pending_enum_ddl(schema_name)) if schema_name else set()
+    pre_ddl = [ddl for ddl in new_type_ddl if ddl not in old_type_ddl]
 
     # analyze differences between declarations
     sql = list()
@@ -761,7 +764,7 @@ def alter(
         # For PostgreSQL: would need COMMENT ON TABLE, but that's not an ALTER TABLE clause
         # Keep MySQL syntax for now (ALTER TABLE ... COMMENT="...")
         sql.append(f'COMMENT="{table_comment}"')
-    return sql, [e for e in external_stores if e not in external_stores_], pre_ddl
+    return sql, [e for e in external_stores if e not in external_stores_], pre_ddl, column_comments
 
 
 def _parse_index_args(args: str) -> list[str]:

src/datajoint/table.py

@@ -322,7 +322,7 @@ def alter(self, prompt=True, context=None):
             context = dict(frame.f_globals, **frame.f_locals)
             del frame
         old_definition = self.describe(context=context)
-        sql, _external_stores, pre_ddl = alter(
+        sql, _external_stores, pre_ddl, column_comments = alter(
             self.definition,
             old_definition,
             context,
@@ -346,10 +346,21 @@ def alter(self, prompt=True, context=None):
                     for ddl in pre_ddl:
                         try:
                             self.connection.query(_substitute_database(ddl, self.database))
-                        except Exception:
-                            # Ignore errors (type may already exist)
-                            pass
+                        except Exception as error:
+                            # Enum type names are content hashes shared by every
+                            # table in the schema using the same value set, so the
+                            # type may already exist. Logged rather than dropped:
+                            # a genuine failure surfaces on the ALTER below.
+                            logger.debug("pre-DDL skipped (%s): %s", error, ddl)
                     self.connection.query(sql)
+                    # Reapply comments. Where they are stored out of line they
+                    # carry the `:type:` prefix heading reads back as
+                    # original_type, without which describe() loses an added
+                    # attribute's declared type and cannot re-parse the table.
+                    for col_name, comment in column_comments.items():
+                        comment_ddl = self.connection.adapter.column_comment_ddl(self.full_table_name, col_name, comment)
+                        if comment_ddl:
+                            self.connection.query(_substitute_database(comment_ddl, self.database))
                 except AccessError:
                     # skip if no create privilege
                     pass

tests/integration/test_multi_backend.py

@@ -204,9 +204,25 @@ class Subject(dj.Manual):
 
     heading = Subject().heading
     assert "status" in heading.names
+    # `type` is the generated type name on PostgreSQL but the full spelling on
+    # MySQL; `original_type` is the definition's own text on both.
+    assert heading["status"].original_type == "enum('active', 'retired', 'transferred')"
 
     # The added column round-trips a value from its own domain.
     Subject.insert1({"subject_id": 1, "species": "mouse", "status": "active"})
     assert (Subject & {"subject_id": 1}).fetch1("status") == "active"
 
+    # Altering again proves the first alter left the type recoverable: describe()
+    # feeds the next alter, so a column whose declared type was not recorded
+    # makes the table permanently un-alterable.
+    Subject.definition = """
+    subject_id : int32
+    ---
+    species : enum('mouse', 'rat')
+    status  : enum('active', 'retired', 'transferred')
+    note = null : varchar(32)
+    """
+    Subject.alter(prompt=False)
+    assert "note" in Subject().heading.names
+
     schema.drop()