From c7d38fb90dbb71e37ba779556108ea4228d41153 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Sat, 17 Oct 2026 17:15:14 -0500 Subject: [PATCH] fix(alter): resolve foreign keys in the table's declaration context With no context argument, Table.alter() resolved the definition's foreign keys in the caller's namespace. Called from any module but the one that declared the table, such as a migration script or a notebook that imports the pipeline module, alter() failed for every table with a foreign key ("Foreign key reference Item could not be resolved") unless the caller passed context= explicitly. Part.alter() already fell back to the declaration context; other tables did not. alter() now uses the table's declaration context when no context is given, and the caller's namespace only for a table that has none. An explicit context still takes precedence. The declaration context is the namespace the definition was written in, and alter() cannot change foreign keys, so it resolves the references the table was declared with. The tests write a pipeline module to a temporary directory, import it, declare its tables in the test schema, and alter them from the test module, on MySQL and PostgreSQL: a table that references another, which fails without the change; its Part table through `-> master`; and the same table with an explicit context that names its parent differently. Each alter adds an attribute, since changing an attribute's type emits MODIFY, which PostgreSQL does not accept. Closes #1602. --- src/datajoint/table.py | 13 ++++ tests/integration/test_alter.py | 101 ++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/src/datajoint/table.py b/src/datajoint/table.py index 9f6784fb3..632b07cdd 100644 --- a/src/datajoint/table.py +++ b/src/datajoint/table.py @@ -315,10 +315,23 @@ def _refresh_lineage(self, context=None): def alter(self, prompt=True, context=None): """ Alter the table definition from self.definition + + Parameters + ---------- + prompt : bool, optional + If True (default), show the ALTER statement and ask for confirmation. + context : dict, optional + Namespace that resolves the definition's foreign key references. + Defaults to the table's declaration context, so the table alters the + same way wherever this is called from; a table without one uses the + caller's namespace. """ if self.connection.in_transaction: raise DataJointError("Cannot update table declaration inside a transaction, e.g. from inside a populate/make call") if context is None: + context = self.declaration_context + if context is None: + # f_back is alter()'s caller, so this cannot move into a helper frame = inspect.currentframe().f_back context = dict(frame.f_globals, **frame.f_locals) del frame diff --git a/tests/integration/test_alter.py b/tests/integration/test_alter.py index fbf074332..8af58e2bb 100644 --- a/tests/integration/test_alter.py +++ b/tests/integration/test_alter.py @@ -1,7 +1,11 @@ +import importlib import re +import sys +import time import pytest +import datajoint as dj from tests import schema as schema_any_module from tests.schema_alter import LOCALS_ALTER, Experiment, Parent @@ -52,3 +56,100 @@ def test_alter_part(self, schema_alter): table=Parent.Grandchild, attribute_sql=r"`grandchild_id` .* DEFAULT NULL[^,]*", ) + + +# A pipeline module that declares its tables at import, like the one in #1602. +# Its schema is activated by the test, on the test's connection. +PIPELINE_MODULE = ''' +import datajoint as dj + +schema = dj.Schema() + + +@schema +class Item(dj.Manual): + definition = """ + item : int32 + """ + + +@schema +class Measurement(dj.Manual): + definition = """ + -> Item + --- + value : float32 + """ + + class Note(dj.Part): + definition = """ + -> master + --- + note : varchar(64) + """ +''' + + +@pytest.fixture +def schema_by_backend(connection_by_backend, db_creds_by_backend): + """Create a fresh schema per test, parameterized across backends.""" + backend = db_creds_by_backend["backend"] + test_id = str(int(time.time() * 1000))[-8:] + schema = dj.Schema(f"djtest_alter_{backend}_{test_id}"[:64], connection=connection_by_backend) + yield schema + schema.drop(prompt=False) + + +@pytest.fixture +def pipeline(schema_by_backend, tmp_path, monkeypatch): + """Import the pipeline module from tmp_path, its tables declared in the test schema.""" + (tmp_path / "alter_pipeline.py").write_text(PIPELINE_MODULE) + monkeypatch.syspath_prepend(str(tmp_path)) + try: + module = importlib.import_module("alter_pipeline") + module.schema.activate(schema_by_backend.database, connection=schema_by_backend.connection) + yield module + finally: + sys.modules.pop("alter_pipeline", None) + + +# Each alter adds an attribute: changing an attribute's type emits MODIFY, which +# PostgreSQL does not accept. Foreign keys resolve before any SQL is generated. + + +def test_alter_from_another_module(pipeline): + """alter() resolves foreign keys in the declaration context, not in the caller's namespace (#1602).""" + # This module has no `Item`, so resolving `-> Item` here would fail + pipeline.Measurement.definition = """ + -> Item + --- + value : float32 + unit = "" : varchar(16) + """ + pipeline.Measurement.alter(prompt=False) + assert pipeline.Measurement.heading.secondary_attributes == ["value", "unit"] + + +def test_alter_part_from_another_module(pipeline): + """A Part table still resolves `-> master` in its declaration context.""" + pipeline.Measurement.Note.definition = """ + -> master + --- + note : varchar(64) + author = "" : varchar(32) + """ + pipeline.Measurement.Note.alter(prompt=False) + assert pipeline.Measurement.Note.heading.secondary_attributes == ["note", "author"] + + +def test_alter_explicit_context(pipeline): + """An explicit context takes precedence over the declaration context.""" + # `Parent` resolves only in the explicit context + pipeline.Measurement.definition = """ + -> Parent + --- + value : float32 + unit = "" : varchar(16) + """ + pipeline.Measurement.alter(prompt=False, context={"Parent": pipeline.Item}) + assert pipeline.Measurement.heading.secondary_attributes == ["value", "unit"]