Skip to content

Commit c7d38fb

Browse files
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.
1 parent a00b89f commit c7d38fb

2 files changed

Lines changed: 114 additions & 0 deletions

File tree

‎src/datajoint/table.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,10 +315,23 @@ def _refresh_lineage(self, context=None):
315315
def alter(self, prompt=True, context=None):
316316
"""
317317
Alter the table definition from self.definition
318+
319+
Parameters
320+
----------
321+
prompt : bool, optional
322+
If True (default), show the ALTER statement and ask for confirmation.
323+
context : dict, optional
324+
Namespace that resolves the definition's foreign key references.
325+
Defaults to the table's declaration context, so the table alters the
326+
same way wherever this is called from; a table without one uses the
327+
caller's namespace.
318328
"""
319329
if self.connection.in_transaction:
320330
raise DataJointError("Cannot update table declaration inside a transaction, e.g. from inside a populate/make call")
321331
if context is None:
332+
context = self.declaration_context
333+
if context is None:
334+
# f_back is alter()'s caller, so this cannot move into a helper
322335
frame = inspect.currentframe().f_back
323336
context = dict(frame.f_globals, **frame.f_locals)
324337
del frame

‎tests/integration/test_alter.py‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
import importlib
12
import re
3+
import sys
4+
import time
25

36
import pytest
47

8+
import datajoint as dj
59

610
from tests import schema as schema_any_module
711
from tests.schema_alter import LOCALS_ALTER, Experiment, Parent
@@ -52,3 +56,100 @@ def test_alter_part(self, schema_alter):
5256
table=Parent.Grandchild,
5357
attribute_sql=r"`grandchild_id` .* DEFAULT NULL[^,]*",
5458
)
59+
60+
61+
# A pipeline module that declares its tables at import, like the one in #1602.
62+
# Its schema is activated by the test, on the test's connection.
63+
PIPELINE_MODULE = '''
64+
import datajoint as dj
65+
66+
schema = dj.Schema()
67+
68+
69+
@schema
70+
class Item(dj.Manual):
71+
definition = """
72+
item : int32
73+
"""
74+
75+
76+
@schema
77+
class Measurement(dj.Manual):
78+
definition = """
79+
-> Item
80+
---
81+
value : float32
82+
"""
83+
84+
class Note(dj.Part):
85+
definition = """
86+
-> master
87+
---
88+
note : varchar(64)
89+
"""
90+
'''
91+
92+
93+
@pytest.fixture
94+
def schema_by_backend(connection_by_backend, db_creds_by_backend):
95+
"""Create a fresh schema per test, parameterized across backends."""
96+
backend = db_creds_by_backend["backend"]
97+
test_id = str(int(time.time() * 1000))[-8:]
98+
schema = dj.Schema(f"djtest_alter_{backend}_{test_id}"[:64], connection=connection_by_backend)
99+
yield schema
100+
schema.drop(prompt=False)
101+
102+
103+
@pytest.fixture
104+
def pipeline(schema_by_backend, tmp_path, monkeypatch):
105+
"""Import the pipeline module from tmp_path, its tables declared in the test schema."""
106+
(tmp_path / "alter_pipeline.py").write_text(PIPELINE_MODULE)
107+
monkeypatch.syspath_prepend(str(tmp_path))
108+
try:
109+
module = importlib.import_module("alter_pipeline")
110+
module.schema.activate(schema_by_backend.database, connection=schema_by_backend.connection)
111+
yield module
112+
finally:
113+
sys.modules.pop("alter_pipeline", None)
114+
115+
116+
# Each alter adds an attribute: changing an attribute's type emits MODIFY, which
117+
# PostgreSQL does not accept. Foreign keys resolve before any SQL is generated.
118+
119+
120+
def test_alter_from_another_module(pipeline):
121+
"""alter() resolves foreign keys in the declaration context, not in the caller's namespace (#1602)."""
122+
# This module has no `Item`, so resolving `-> Item` here would fail
123+
pipeline.Measurement.definition = """
124+
-> Item
125+
---
126+
value : float32
127+
unit = "" : varchar(16)
128+
"""
129+
pipeline.Measurement.alter(prompt=False)
130+
assert pipeline.Measurement.heading.secondary_attributes == ["value", "unit"]
131+
132+
133+
def test_alter_part_from_another_module(pipeline):
134+
"""A Part table still resolves `-> master` in its declaration context."""
135+
pipeline.Measurement.Note.definition = """
136+
-> master
137+
---
138+
note : varchar(64)
139+
author = "" : varchar(32)
140+
"""
141+
pipeline.Measurement.Note.alter(prompt=False)
142+
assert pipeline.Measurement.Note.heading.secondary_attributes == ["note", "author"]
143+
144+
145+
def test_alter_explicit_context(pipeline):
146+
"""An explicit context takes precedence over the declaration context."""
147+
# `Parent` resolves only in the explicit context
148+
pipeline.Measurement.definition = """
149+
-> Parent
150+
---
151+
value : float32
152+
unit = "" : varchar(16)
153+
"""
154+
pipeline.Measurement.alter(prompt=False, context={"Parent": pipeline.Item})
155+
assert pipeline.Measurement.heading.secondary_attributes == ["value", "unit"]

0 commit comments

Comments
 (0)