Skip to content

Commit a00b89f

Browse files
Merge pull request #1579 from datajoint/fix/fk-core-type-marker
fix: a column copied by a foreign key keeps the parent's core type
2 parents 43f5c26 + 8bf3b67 commit a00b89f

3 files changed

Lines changed: 154 additions & 4 deletions

File tree

‎src/datajoint/declare.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ def compile_foreign_key(
203203
adapter,
204204
fk_attribute_map: dict[str, tuple[str, str]] | None = None,
205205
fk_index_candidates: list[list[str]] | None = None,
206+
column_comments: dict[str, str] | None = None,
206207
) -> None:
207208
"""
208209
Parse a foreign key line and update declaration components.
@@ -233,6 +234,11 @@ def compile_foreign_key(
233234
candidate supporting index; the redundancy/coverage decision is made
234235
post-parse (once the full primary key and index list are known).
235236
Updated in place.
237+
column_comments : dict, optional
238+
Mapping of ``column -> comment`` for backends that set column comments
239+
out of line (PostgreSQL's ``COMMENT ON COLUMN``). Each copied column's
240+
comment, with its core-type marker (e.g. ``:int32:``), is added so the
241+
child keeps the parent's declared type. Updated in place.
236242
237243
Raises
238244
------
@@ -288,14 +294,17 @@ def compile_foreign_key(
288294
# Enum type names start with "enum_" (generated hash-based names)
289295
if sql_type.startswith("enum_") and adapter.backend == "postgresql":
290296
sql_type = f"{adapter.quote_identifier(ref.database)}.{adapter.quote_identifier(sql_type)}"
297+
comment = parent_attr.sql_comment
291298
col_def = adapter.format_column_definition(
292299
name=attr,
293300
sql_type=sql_type,
294301
nullable=is_nullable,
295302
default=None,
296-
comment=parent_attr.sql_comment,
303+
comment=comment,
297304
)
298305
attr_sql.append(col_def)
306+
if comment and column_comments is not None:
307+
column_comments[attr] = comment
299308

300309
# Track FK attribute mapping for lineage: child_attr -> (parent_table, parent_attr)
301310
if fk_attribute_map is not None:
@@ -459,6 +468,7 @@ def add_attribute(line: str, in_key: bool) -> None:
459468
adapter,
460469
fk_attribute_map,
461470
fk_index_candidates,
471+
column_comments,
462472
)
463473
elif re.match(r"^(unique\s+)?index\s*\(.*\)\s*(#.*)?$", line, re.I): # index
464474
compile_index(re.sub(r"\s*#.*$", "", line), index_sql, adapter)

‎src/datajoint/heading.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,23 @@ def sql_comment(self) -> str:
127127
"""
128128
Return the full SQL comment including type markers.
129129
130+
A core type (``int32``, ``uuid``, ``float64``, ...) is recorded in the column
131+
comment as ``:type:`` so it can be reconstructed from the database; heading
132+
parsing strips the marker into ``original_type``. Restoring it here keeps the
133+
core type on columns copied by a foreign key.
134+
130135
Returns
131136
-------
132137
str
133-
Comment with optional ``:uuid:`` prefix.
138+
Comment with the core-type prefix, e.g. ``:int32:``, when there is one.
134139
"""
135-
# UUID info is stored in the comment for reconstruction
136-
return (":uuid:" if self.uuid else "") + (self.comment or "")
140+
if self.original_type:
141+
marker = f":{self.original_type}:"
142+
elif self.uuid:
143+
marker = ":uuid:"
144+
else:
145+
marker = ""
146+
return marker + (self.comment or "")
137147

138148
@property
139149
def sql(self) -> str:
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
"""
2+
A column copied by a foreign key keeps the parent's core type.
3+
4+
Core types (``int32``, ``int16``, ``uuid``, ...) are recorded in the column comment as
5+
``:type:``. A foreign key copies the parent's primary-key columns into the child; the copy
6+
must carry the same marker, or the child's heading reports the native SQL type and every
7+
table derived from it (a jobs table, a further child) inherits the loss.
8+
"""
9+
10+
import logging
11+
12+
import pytest
13+
14+
import datajoint as dj
15+
16+
17+
class _Records(logging.Handler):
18+
def __init__(self):
19+
super().__init__(level=logging.WARNING)
20+
self.messages = []
21+
22+
def emit(self, record):
23+
self.messages.append(record.getMessage())
24+
25+
26+
@pytest.mark.backend_agnostic
27+
def test_fk_columns_keep_core_types(connection_by_backend, backend, prefix):
28+
schema = dj.Schema(f"{prefix}_fk_core_types_{backend}", connection=connection_by_backend)
29+
try:
30+
31+
@schema
32+
class Subject(dj.Manual):
33+
definition = """
34+
subject_id : int32
35+
"""
36+
37+
@schema
38+
class Session(dj.Manual):
39+
definition = """
40+
-> Subject
41+
session_idx : int16
42+
"""
43+
44+
@schema
45+
class Summary(dj.Manual):
46+
definition = """
47+
-> Session
48+
---
49+
n : int32
50+
"""
51+
52+
assert Subject.heading["subject_id"].original_type == "int32"
53+
# inherited one level down
54+
assert Session.heading["subject_id"].original_type == "int32"
55+
assert Session.heading["session_idx"].original_type == "int16"
56+
# inherited two levels down
57+
assert Summary.heading["subject_id"].original_type == "int32"
58+
assert Summary.heading["session_idx"].original_type == "int16"
59+
# the comment text itself is unchanged
60+
assert Summary.heading["subject_id"].comment == Subject.heading["subject_id"].comment
61+
finally:
62+
schema.drop(prompt=False)
63+
64+
65+
@pytest.mark.backend_agnostic
66+
def test_fk_columns_keep_uuid(connection_by_backend, backend, prefix):
67+
schema = dj.Schema(f"{prefix}_fk_core_types_uuid_{backend}", connection=connection_by_backend)
68+
try:
69+
70+
@schema
71+
class Item(dj.Manual):
72+
definition = """
73+
item_id : uuid # identifier of the item
74+
"""
75+
76+
@schema
77+
class Tag(dj.Manual):
78+
definition = """
79+
-> Item
80+
tag : varchar(16)
81+
"""
82+
83+
assert Tag.heading["item_id"].uuid
84+
assert Tag.heading["item_id"].original_type == "uuid"
85+
assert Tag.heading["item_id"].comment == "identifier of the item"
86+
finally:
87+
schema.drop(prompt=False)
88+
89+
90+
@pytest.mark.backend_agnostic
91+
def test_jobs_table_keeps_inherited_core_types(connection_by_backend, backend, prefix):
92+
"""A jobs table built from inherited keys declares core types and warns about nothing."""
93+
schema = dj.Schema(f"{prefix}_fk_core_types_jobs_{backend}", connection=connection_by_backend)
94+
records = _Records()
95+
logger = logging.getLogger("datajoint")
96+
try:
97+
98+
@schema
99+
class Subject(dj.Manual):
100+
definition = """
101+
subject_id : int32
102+
"""
103+
104+
@schema
105+
class Session(dj.Manual):
106+
definition = """
107+
-> Subject
108+
session_idx : int16
109+
"""
110+
111+
@schema
112+
class Summary(dj.Computed):
113+
definition = """
114+
-> Session
115+
---
116+
n : int32
117+
"""
118+
119+
def make(self, key):
120+
self.insert1(dict(key, n=1))
121+
122+
logger.addHandler(records)
123+
jobs = Summary.jobs
124+
assert jobs.heading["subject_id"].original_type == "int32"
125+
assert jobs.heading["session_idx"].original_type == "int16"
126+
native = [m for m in records.messages if m.startswith("Native type")]
127+
assert native == [], native
128+
finally:
129+
logger.removeHandler(records)
130+
schema.drop(prompt=False)

0 commit comments

Comments
 (0)