Skip to content

Commit 49fd1e4

Browse files
fix: validate attribute types at declaration and insert (#1527, #1528, #1529, #1530)
Four bugs with one shape: the type system accepted spellings it could not honour, and failed late — at the driver or the server — instead of at declaration. #1527 A native blob column accepted any object. DataJoint passed it to the driver untouched; PyMySQL has no encoder for ndarray and falls back to str(value), so an array was stored as its text repr, elided in the middle for large arrays, with no error on insert or on fetch. __make_placeholder now requires bytes for a codec-less blob attribute. The declaration warning for native blob types now says the column stores raw bytes and names <blob>, rather than the generic portability note. #1528 decimal(M,D) carrying `unsigned` or `zerofill`, spelled `dec`/`fixed`, or given a single argument was rejected outright, where 0.14.x accepted it. 2.x promoted decimal to a core type with a strict pattern and left the old permissive branch under the name NUMERIC, which kept `numeric` and dropped `decimal` — so `numeric(2,2) unsigned` passed while the canonical spelling of the same SQL type did not. NUMERIC covers all four aliases again; the core DECIMAL pattern still wins for decimal(M,D). Heading derives its `numeric` flag from NUMERIC too, so a modified decimal is no longer classified as neither numeric nor string. #1529 NATIVE_TO_CORE_TYPE mapped unsigned integers to uint8/uint16/uint24/ uint32/uint64 and mediumint to int24, none of which exist as core types. Phase 2 migration wrote :uint32: markers that Heading then silently dropped. Unsigned columns now widen to the next signed type that holds their range, as the migration guide already documented. bigint unsigned has no lossless target and logs a warning. #1530 match_type used re.match, and the $ in the INTEGER pattern bound to the `serial` alternative only, so `int24`, `tinyinteger` and `intbanana` all classified as native integers and were emitted into the DDL verbatim. Now fullmatch, with the INTEGER alternation anchored as a whole. Also: bare `blob` is a MySQL type and was rejected while the sized variants were accepted, though migrate.BLOB_TYPES already recognized it. Found by the new cross-reference test. tests/unit/test_type_patterns.py asserts that every type named in NATIVE_TO_CORE_TYPE round-trips through match_type, that near-miss spellings raise, and that the native spellings still accepted are not regressed by the strictness change.
1 parent 8d05d98 commit 49fd1e4

6 files changed

Lines changed: 294 additions & 15 deletions

File tree

‎src/datajoint/declare.py‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,17 @@
6464
# Core DataJoint types
6565
**{name.upper(): pattern for name, (pattern, _) in CORE_TYPES.items()},
6666
# Native SQL types (passthrough with warning for non-standard use)
67-
INTEGER=r"((tiny|small|medium|big|)int|integer)(\s*\(.+\))?(\s+unsigned)?(\s+auto_increment)?|serial$",
68-
NUMERIC=r"numeric(\s*\(.+\))?(\s+unsigned)?$", # numeric is SQL alias, use decimal instead
67+
INTEGER=r"(((tiny|small|medium|big|)int|integer)(\s*\(.+\))?(\s+unsigned)?(\s+auto_increment)?|serial)$",
68+
# decimal/numeric/dec/fixed are the same SQL type. The canonical core spelling
69+
# decimal(M,D) is matched above; anything carrying a modifier or an alias falls
70+
# through to here and passes as a native type, as int/float unsigned do.
71+
NUMERIC=r"(decimal|numeric|dec|fixed)(\s*\(.+\))?(\s+unsigned)?(\s+zerofill)?$",
6972
FLOAT=r"(double|float|real)(\s*\(.+\))?(\s+unsigned)?$",
7073
STRING=r"(var)?char\s*\(.+\)$", # Catches char/varchar not matched by core types
7174
TEMPORAL=r"(time|timestamp|year)(\s*\(.+\))?$", # time, timestamp, year (not date/datetime)
72-
NATIVE_BLOB=r"(tiny|small|medium|long)blob$", # Specific blob variants
75+
# Size prefix is optional: bare `blob` is a MySQL type in its own right, and is
76+
# already recognized by migrate.BLOB_TYPES.
77+
NATIVE_BLOB=r"(tiny|small|medium|long)?blob$",
7378
NATIVE_TEXT=r"(tiny|small|medium|long)?text$", # Native text types (not portable)
7479
# Codecs use angle brackets
7580
CODEC=r"<.+>$",
@@ -107,8 +112,12 @@ def match_type(attribute_type: str) -> str:
107112
DataJointError
108113
If the type string doesn't match any known pattern.
109114
"""
115+
# fullmatch, not match: a declared type must be consumed in its entirety. With
116+
# a prefix match, a misspelling such as `int24` or `tinyinteger` classifies as a
117+
# native integer and is emitted into the DDL verbatim, so the failure surfaces as
118+
# a SQL syntax error from the server rather than as an unsupported-type error here.
110119
try:
111-
return next(category for category, pattern in TYPE_PATTERN.items() if pattern.match(attribute_type))
120+
return next(category for category, pattern in TYPE_PATTERN.items() if pattern.fullmatch(attribute_type))
112121
except StopIteration:
113122
raise DataJointError("Unsupported attribute type {type}".format(type=attribute_type))
114123

@@ -913,6 +922,16 @@ def compile_attribute(
913922
# Core types and Codecs are recorded in comment for reconstruction
914923
match["comment"] = ":{type}:{comment}".format(**match)
915924
substitute_special_type(match, category, foreign_key_sql, context, adapter)
925+
elif category == "NATIVE_BLOB":
926+
# A native blob is a raw binary column, not a DataJoint blob. Say so plainly:
927+
# the generic portability warning below reads as cosmetic, and users migrating
928+
# from pre-2.0 expect `longblob` to serialize objects for them.
929+
logger.warning(
930+
f"Native type '{match['type']}' is used in attribute '{match['name']}'. "
931+
"It stores raw bytes only — values are not serialized on write or "
932+
"deserialized on read. Use '<blob>' to store arrays and other Python "
933+
"objects, or 'bytes' if raw binary is intended."
934+
)
916935
elif category in NATIVE_TYPES:
917936
# Native type - warn user
918937
logger.warning(

‎src/datajoint/heading.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,11 @@ def _init_from_database(self) -> None:
484484
in_key=(attr["key"] == "PRI"),
485485
nullable=attr["nullable"], # Already boolean from parse_column_info
486486
autoincrement=bool(re.search(r"auto_increment", attr["extra"], flags=re.I)),
487-
numeric=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("DECIMAL", "INTEGER", "FLOAT")),
487+
# NOTE: these use prefix matching, unlike match_type() which requires a
488+
# full match. attr["type"] is reported by the server and is not
489+
# normalized, so PostgreSQL spellings such as "double precision" and
490+
# "timestamp without time zone" are matched on their leading word.
491+
numeric=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("DECIMAL", "NUMERIC", "INTEGER", "FLOAT")),
488492
string=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("ENUM", "TEMPORAL", "STRING")),
489493
is_blob=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("BYTES", "NATIVE_BLOB")),
490494
uuid=False,

‎src/datajoint/migrate.py‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,18 +61,25 @@
6161
# Column Type Migration (Phase 2)
6262
# =============================================================================
6363

64-
# Mapping from MySQL native types to DataJoint core types
64+
# Mapping from MySQL native types to DataJoint core types.
65+
#
66+
# Every value here must be a type that `declare.match_type` accepts: the values are
67+
# written into column comments as `:<value>:` markers and read back by
68+
# `heading.Heading._init_from_database`. DataJoint 2.0 provides no unsigned integer
69+
# types, so unsigned columns widen to the next signed type that holds their full range.
70+
# See `test_migrate_core_types_are_declarable` for the assertion that enforces this.
6571
NATIVE_TO_CORE_TYPE = {
66-
# Unsigned integers
67-
"tinyint unsigned": "uint8",
68-
"smallint unsigned": "uint16",
69-
"mediumint unsigned": "uint24",
70-
"int unsigned": "uint32",
71-
"bigint unsigned": "uint64",
72+
# Unsigned integers widen to the next signed type. bigint unsigned is the one case
73+
# with no lossless target: values above 2**63-1 do not fit in int64.
74+
"tinyint unsigned": "int16",
75+
"smallint unsigned": "int32",
76+
"mediumint unsigned": "int32",
77+
"int unsigned": "int64",
78+
"bigint unsigned": "int64",
7279
# Signed integers
7380
"tinyint": "int8",
7481
"smallint": "int16",
75-
"mediumint": "int24",
82+
"mediumint": "int32", # int24 is not a core type
7683
"int": "int32",
7784
"bigint": "int64",
7885
# Floats
@@ -191,6 +198,13 @@ def analyze_columns(schema: Schema) -> dict:
191198
# Handle numeric types
192199
elif lookup_type in NATIVE_TO_CORE_TYPE:
193200
col_info["core_type"] = NATIVE_TO_CORE_TYPE[lookup_type]
201+
if lookup_type == "bigint unsigned":
202+
logger.warning(
203+
f"Column `{col_info['table']}`.`{col_info['column']}` is "
204+
"`bigint unsigned` and will be labeled `int64`. DataJoint 2.0 "
205+
"provides no unsigned integer types; values above 2**63-1 do "
206+
"not fit in int64. Verify the stored range before migrating."
207+
)
194208
result["needs_migration"].append(col_info)
195209
# Types that don't need migration (varchar, date, datetime, json, etc.)
196210
# are silently skipped
@@ -211,7 +225,7 @@ def migrate_columns(
211225
212226
Migrates:
213227
214-
- Numeric types: int unsigned → :uint32:, smallint → :int16:, etc.
228+
- Numeric types: int unsigned → :int64:, smallint → :int16:, etc.
215229
- Blob types: longblob → :<blob>:
216230
217231
Does NOT migrate external storage columns (external-*, attach@*,

‎src/datajoint/table.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1415,7 +1415,19 @@ def __make_placeholder(self, name, value, ignore_extra_fields=False, row=None):
14151415
# Numeric - convert to string
14161416
elif attr.numeric:
14171417
value = str(int(value) if isinstance(value, (bool, np.bool_)) else value)
1418-
# Blob - pass through as bytes (use <blob> for automatic serialization)
1418+
# Native blob - raw bytes only. Anything else is rejected here rather than
1419+
# handed to the driver: PyMySQL has no encoder for objects such as ndarray
1420+
# and falls back to str(value), which stores the text repr of the object
1421+
# (elided in the middle, for a large array) with no error on insert or on
1422+
# fetch. Use <blob> for serialization.
1423+
elif attr.is_blob and attr.codec is None:
1424+
if not isinstance(value, (bytes, bytearray, memoryview)):
1425+
raise DataJointError(
1426+
f"Attribute `{name}` is declared as the native binary type "
1427+
f"`{attr.type}`, which stores raw bytes, but a value of type "
1428+
f"`{type(value).__name__}` was given. Declare the attribute as "
1429+
"'<blob>' to store arrays and other Python objects, or pass bytes."
1430+
)
14191431

14201432
return name, placeholder, value
14211433

‎tests/integration/test_declare.py‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,3 +470,73 @@ class Metadata(dj.Lookup):
470470
# Description should show just the secondary attribute
471471
assert "info" in description
472472
# _singleton is hidden, implementation detail
473+
474+
475+
def test_native_blob_rejects_non_bytes(schema_any):
476+
"""
477+
A native blob column stores raw bytes. Handing it an object silently stored
478+
str(object) before the guard in __make_placeholder; it must raise instead.
479+
"""
480+
import numpy as np
481+
482+
@schema_any
483+
class RawBinary(dj.Manual):
484+
definition = """
485+
id : int32
486+
---
487+
data : longblob
488+
"""
489+
490+
with pytest.raises(dj.DataJointError, match="native binary type"):
491+
RawBinary.insert1({"id": 1, "data": np.arange(500, dtype="float32")})
492+
with pytest.raises(dj.DataJointError, match="native binary type"):
493+
RawBinary.insert1({"id": 2, "data": [1, 2, 3]})
494+
with pytest.raises(dj.DataJointError, match="native binary type"):
495+
RawBinary.insert1({"id": 3, "data": "text"})
496+
497+
# bytes are what the column is for, and must round-trip untouched
498+
RawBinary.insert1({"id": 4, "data": b"\x00\x01\x02"})
499+
assert (RawBinary & "id=4").fetch1("data") == b"\x00\x01\x02"
500+
assert len(RawBinary()) == 1 # none of the rejected inserts landed
501+
RawBinary.drop_quick()
502+
503+
504+
def test_bare_blob_datatype(schema_any):
505+
"""`blob` without a size prefix is a MySQL type in its own right."""
506+
507+
@schema_any
508+
class BareBlob(dj.Manual):
509+
definition = """
510+
id : int32
511+
---
512+
data : blob
513+
"""
514+
515+
BareBlob.insert1({"id": 1, "data": b"abc"})
516+
assert (BareBlob & "id=1").fetch1("data") == b"abc"
517+
BareBlob.drop_quick()
518+
519+
520+
def test_decimal_with_modifiers(schema_any):
521+
"""
522+
decimal(M,D) carrying `unsigned`, or spelled with a single argument, was valid in
523+
0.14.x and must remain declarable as a native type.
524+
"""
525+
from decimal import Decimal
526+
527+
@schema_any
528+
class Proportion(dj.Manual):
529+
definition = """
530+
id : int32
531+
---
532+
on_proportion : decimal(2, 2) unsigned
533+
scale : decimal(5)
534+
"""
535+
536+
Proportion.insert1({"id": 1, "on_proportion": Decimal("0.25"), "scale": 42})
537+
row = (Proportion & "id=1").fetch1()
538+
assert row["on_proportion"] == Decimal("0.25")
539+
assert int(row["scale"]) == 42
540+
# the modified decimal must be recognized as numeric, not fall through as unsupported
541+
assert Proportion.heading["on_proportion"].numeric
542+
Proportion.drop_quick()

‎tests/unit/test_type_patterns.py‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
"""
2+
Type-pattern consistency and strictness.
3+
4+
These tests cover the seam between the three places that name a type: the core type
5+
table in ``declare``, the native passthrough patterns beside it, and the legacy mapping
6+
in ``migrate``. Each of the bugs exercised here was a spelling that one of those three
7+
accepted and another did not.
8+
"""
9+
10+
import pytest
11+
12+
from datajoint.declare import CORE_TYPES, TYPE_PATTERN, match_type
13+
from datajoint.errors import DataJointError
14+
from datajoint.migrate import NATIVE_TO_CORE_TYPE
15+
16+
17+
class TestCrossReferences:
18+
"""Every type named in one module must be recognized by the others."""
19+
20+
def test_migrate_core_types_are_declarable(self):
21+
"""
22+
Values in NATIVE_TO_CORE_TYPE are written into column comments as ``:type:``
23+
markers and read back by Heading. A value that match_type rejects produces a
24+
marker that cannot be resolved on load.
25+
"""
26+
for native, core in NATIVE_TO_CORE_TYPE.items():
27+
if core.startswith("<"):
28+
continue # codecs are resolved by the codec registry, not match_type
29+
assert match_type(core), f"{native!r} maps to {core!r}, which match_type rejects"
30+
31+
def test_migrate_native_types_are_recognized(self):
32+
"""The keys are types read from a legacy database and must classify too."""
33+
for native in NATIVE_TO_CORE_TYPE:
34+
assert match_type(native)
35+
36+
def test_core_type_names_are_declarable(self):
37+
"""Each core type's own canonical spelling must classify as that core type."""
38+
canonical = {
39+
"float32": "float32",
40+
"float64": "float64",
41+
"int64": "int64",
42+
"int32": "int32",
43+
"int16": "int16",
44+
"int8": "int8",
45+
"bool": "bool",
46+
"uuid": "uuid",
47+
"json": "json",
48+
"bytes": "bytes",
49+
"date": "date",
50+
"datetime": "datetime",
51+
"char": "char(8)",
52+
"varchar": "varchar(8)",
53+
"enum": "enum('a','b')",
54+
"decimal": "decimal(6,2)",
55+
}
56+
assert set(canonical) == set(CORE_TYPES), "CORE_TYPES changed; update this map"
57+
for name, spelling in canonical.items():
58+
assert match_type(spelling) == name.upper()
59+
60+
61+
class TestStrictness:
62+
"""A declared type must be consumed in its entirety, not merely prefixed."""
63+
64+
@pytest.mark.parametrize(
65+
"spelling",
66+
[
67+
"int24", # plausible-looking, and formerly emitted by migrate.py
68+
"intbanana",
69+
"integerish",
70+
"tinyinteger",
71+
"uint32", # unsigned core types are deliberately not provided
72+
"uint8",
73+
"float64x",
74+
"completely_invalid_type_xyz",
75+
],
76+
)
77+
def test_near_miss_spellings_are_rejected(self, spelling):
78+
with pytest.raises(DataJointError, match="Unsupported attribute type"):
79+
match_type(spelling)
80+
81+
def test_serial_alternative_stays_anchored(self):
82+
"""`serial` is an alternative inside the INTEGER pattern; it must not leak."""
83+
assert match_type("serial") == "INTEGER"
84+
with pytest.raises(DataJointError):
85+
match_type("serialize")
86+
87+
88+
class TestNumericAliases:
89+
"""decimal/numeric/dec/fixed are one SQL type and must be treated alike."""
90+
91+
def test_canonical_decimal_is_a_core_type(self):
92+
assert match_type("decimal(6,2)") == "DECIMAL"
93+
assert match_type("decimal(2, 2)") == "DECIMAL"
94+
95+
@pytest.mark.parametrize(
96+
"spelling",
97+
[
98+
"decimal(2,2) unsigned",
99+
"decimal(2, 2) unsigned",
100+
"DECIMAL(2,2) UNSIGNED",
101+
"decimal(2,2) zerofill",
102+
"decimal(5)",
103+
"decimal",
104+
"dec(2,2)",
105+
"fixed(2,2)",
106+
"numeric(2,2)",
107+
"numeric(2,2) unsigned",
108+
],
109+
)
110+
def test_modified_and_aliased_forms_pass_as_native(self, spelling):
111+
"""
112+
These were valid in 0.14.x and must remain declarable. They pass as native
113+
types with a warning rather than as the core decimal type.
114+
"""
115+
assert match_type(spelling) == "NUMERIC"
116+
117+
def test_numeric_is_recognized_as_numeric_by_heading(self):
118+
"""
119+
Heading derives its `numeric` flag from the same patterns. A decimal carrying a
120+
modifier must not fall through as neither numeric nor string.
121+
"""
122+
for spelling in ("decimal(2,2) unsigned", "numeric(6,2)", "decimal(5)"):
123+
assert any(TYPE_PATTERN[t].match(spelling) for t in ("DECIMAL", "NUMERIC", "INTEGER", "FLOAT"))
124+
125+
126+
class TestNativeSpellingsStillAccepted:
127+
"""Guard against the strictness change rejecting legitimate native types."""
128+
129+
@pytest.mark.parametrize(
130+
"spelling,category",
131+
[
132+
("int", "INTEGER"),
133+
("int(11)", "INTEGER"),
134+
("int unsigned", "INTEGER"),
135+
("int(11) unsigned", "INTEGER"),
136+
("bigint unsigned auto_increment", "INTEGER"),
137+
("integer", "INTEGER"),
138+
("tinyint", "INTEGER"),
139+
("smallint", "INTEGER"),
140+
("double", "FLOAT"),
141+
("float", "FLOAT"),
142+
("real", "FLOAT"),
143+
("double unsigned", "FLOAT"),
144+
("varchar(255)", "VARCHAR"),
145+
("char(4)", "CHAR"),
146+
("timestamp", "TEMPORAL"),
147+
("time", "TEMPORAL"),
148+
("year", "TEMPORAL"),
149+
("longblob", "NATIVE_BLOB"),
150+
("mediumblob", "NATIVE_BLOB"),
151+
("blob", "NATIVE_BLOB"), # bare blob is a MySQL type too
152+
("longtext", "NATIVE_TEXT"),
153+
("text", "NATIVE_TEXT"),
154+
("<blob>", "CODEC"),
155+
("<attach>", "CODEC"),
156+
("<blob@store>", "CODEC"),
157+
],
158+
)
159+
def test_spelling_classifies(self, spelling, category):
160+
assert match_type(spelling) == category

0 commit comments

Comments
 (0)