diff --git a/src/datajoint/declare.py b/src/datajoint/declare.py index dfd4c85df..4fc127921 100644 --- a/src/datajoint/declare.py +++ b/src/datajoint/declare.py @@ -64,12 +64,17 @@ # Core DataJoint types **{name.upper(): pattern for name, (pattern, _) in CORE_TYPES.items()}, # Native SQL types (passthrough with warning for non-standard use) - INTEGER=r"((tiny|small|medium|big|)int|integer)(\s*\(.+\))?(\s+unsigned)?(\s+auto_increment)?|serial$", - NUMERIC=r"numeric(\s*\(.+\))?(\s+unsigned)?$", # numeric is SQL alias, use decimal instead + INTEGER=r"(((tiny|small|medium|big|)int|integer)(\s*\(.+\))?(\s+unsigned)?(\s+auto_increment)?|serial)$", + # decimal/numeric/dec/fixed are the same SQL type. The canonical core spelling + # decimal(M,D) is matched above; anything carrying a modifier or an alias falls + # through to here and passes as a native type, as int/float unsigned do. + NUMERIC=r"(decimal|numeric|dec|fixed)(\s*\(.+\))?(\s+unsigned)?(\s+zerofill)?$", FLOAT=r"(double|float|real)(\s*\(.+\))?(\s+unsigned)?$", STRING=r"(var)?char\s*\(.+\)$", # Catches char/varchar not matched by core types TEMPORAL=r"(time|timestamp|year)(\s*\(.+\))?$", # time, timestamp, year (not date/datetime) - NATIVE_BLOB=r"(tiny|small|medium|long)blob$", # Specific blob variants + # Size prefix is optional: bare `blob` is a MySQL type in its own right, and is + # already recognized by migrate.BLOB_TYPES. + NATIVE_BLOB=r"(tiny|small|medium|long)?blob$", NATIVE_TEXT=r"(tiny|small|medium|long)?text$", # Native text types (not portable) # Codecs use angle brackets CODEC=r"<.+>$", @@ -107,8 +112,12 @@ def match_type(attribute_type: str) -> str: DataJointError If the type string doesn't match any known pattern. """ + # fullmatch, not match: a declared type must be consumed in its entirety. With + # a prefix match, a misspelling such as `int24` or `tinyinteger` classifies as a + # native integer and is emitted into the DDL verbatim, so the failure surfaces as + # a SQL syntax error from the server rather than as an unsupported-type error here. try: - return next(category for category, pattern in TYPE_PATTERN.items() if pattern.match(attribute_type)) + return next(category for category, pattern in TYPE_PATTERN.items() if pattern.fullmatch(attribute_type)) except StopIteration: raise DataJointError("Unsupported attribute type {type}".format(type=attribute_type)) @@ -913,6 +922,16 @@ def compile_attribute( # Core types and Codecs are recorded in comment for reconstruction match["comment"] = ":{type}:{comment}".format(**match) substitute_special_type(match, category, foreign_key_sql, context, adapter) + elif category == "NATIVE_BLOB": + # A native blob is a raw binary column, not a DataJoint blob. Say so plainly: + # the generic portability warning below reads as cosmetic, and users migrating + # from pre-2.0 expect `longblob` to serialize objects for them. + logger.warning( + f"Native type '{match['type']}' is used in attribute '{match['name']}'. " + "It stores raw bytes only — values are not serialized on write or " + "deserialized on read. Use '' to store arrays and other Python " + "objects, or 'bytes' if raw binary is intended." + ) elif category in NATIVE_TYPES: # Native type - warn user logger.warning( diff --git a/src/datajoint/heading.py b/src/datajoint/heading.py index 722c412f4..a405ea27d 100644 --- a/src/datajoint/heading.py +++ b/src/datajoint/heading.py @@ -484,7 +484,11 @@ def _init_from_database(self) -> None: in_key=(attr["key"] == "PRI"), nullable=attr["nullable"], # Already boolean from parse_column_info autoincrement=bool(re.search(r"auto_increment", attr["extra"], flags=re.I)), - numeric=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("DECIMAL", "INTEGER", "FLOAT")), + # NOTE: these use prefix matching, unlike match_type() which requires a + # full match. attr["type"] is reported by the server and is not + # normalized, so PostgreSQL spellings such as "double precision" and + # "timestamp without time zone" are matched on their leading word. + numeric=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("DECIMAL", "NUMERIC", "INTEGER", "FLOAT")), string=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("ENUM", "TEMPORAL", "STRING")), is_blob=any(TYPE_PATTERN[t].match(attr["type"]) for t in ("BYTES", "NATIVE_BLOB")), uuid=False, diff --git a/src/datajoint/migrate.py b/src/datajoint/migrate.py index 1f174ccfd..f20d6ee43 100644 --- a/src/datajoint/migrate.py +++ b/src/datajoint/migrate.py @@ -61,18 +61,25 @@ # Column Type Migration (Phase 2) # ============================================================================= -# Mapping from MySQL native types to DataJoint core types +# Mapping from MySQL native types to DataJoint core types. +# +# Every value here must be a type that `declare.match_type` accepts: the values are +# written into column comments as `::` markers and read back by +# `heading.Heading._init_from_database`. DataJoint 2.0 provides no unsigned integer +# types, so unsigned columns widen to the next signed type that holds their full range. +# See `test_migrate_core_types_are_declarable` for the assertion that enforces this. NATIVE_TO_CORE_TYPE = { - # Unsigned integers - "tinyint unsigned": "uint8", - "smallint unsigned": "uint16", - "mediumint unsigned": "uint24", - "int unsigned": "uint32", - "bigint unsigned": "uint64", + # Unsigned integers widen to the next signed type. bigint unsigned is the one case + # with no lossless target: values above 2**63-1 do not fit in int64. + "tinyint unsigned": "int16", + "smallint unsigned": "int32", + "mediumint unsigned": "int32", + "int unsigned": "int64", + "bigint unsigned": "int64", # Signed integers "tinyint": "int8", "smallint": "int16", - "mediumint": "int24", + "mediumint": "int32", # int24 is not a core type "int": "int32", "bigint": "int64", # Floats @@ -191,6 +198,13 @@ def analyze_columns(schema: Schema) -> dict: # Handle numeric types elif lookup_type in NATIVE_TO_CORE_TYPE: col_info["core_type"] = NATIVE_TO_CORE_TYPE[lookup_type] + if lookup_type == "bigint unsigned": + logger.warning( + f"Column `{col_info['table']}`.`{col_info['column']}` is " + "`bigint unsigned` and will be labeled `int64`. DataJoint 2.0 " + "provides no unsigned integer types; values above 2**63-1 do " + "not fit in int64. Verify the stored range before migrating." + ) result["needs_migration"].append(col_info) # Types that don't need migration (varchar, date, datetime, json, etc.) # are silently skipped @@ -211,7 +225,7 @@ def migrate_columns( Migrates: - - Numeric types: int unsigned → :uint32:, smallint → :int16:, etc. + - Numeric types: int unsigned → :int64:, smallint → :int16:, etc. - Blob types: longblob → :: Does NOT migrate external storage columns (external-*, attach@*, diff --git a/src/datajoint/table.py b/src/datajoint/table.py index 6406acd24..a97616133 100644 --- a/src/datajoint/table.py +++ b/src/datajoint/table.py @@ -1415,7 +1415,19 @@ def __make_placeholder(self, name, value, ignore_extra_fields=False, row=None): # Numeric - convert to string elif attr.numeric: value = str(int(value) if isinstance(value, (bool, np.bool_)) else value) - # Blob - pass through as bytes (use for automatic serialization) + # Native blob - raw bytes only. Anything else is rejected here rather than + # handed to the driver: PyMySQL has no encoder for objects such as ndarray + # and falls back to str(value), which stores the text repr of the object + # (elided in the middle, for a large array) with no error on insert or on + # fetch. Use for serialization. + elif attr.is_blob and attr.codec is None: + if not isinstance(value, (bytes, bytearray, memoryview)): + raise DataJointError( + f"Attribute `{name}` is declared as the native binary type " + f"`{attr.type}`, which stores raw bytes, but a value of type " + f"`{type(value).__name__}` was given. Declare the attribute as " + "'' to store arrays and other Python objects, or pass bytes." + ) return name, placeholder, value diff --git a/tests/integration/test_declare.py b/tests/integration/test_declare.py index 19e711e96..7db8c5ce6 100644 --- a/tests/integration/test_declare.py +++ b/tests/integration/test_declare.py @@ -470,3 +470,73 @@ class Metadata(dj.Lookup): # Description should show just the secondary attribute assert "info" in description # _singleton is hidden, implementation detail + + +def test_native_blob_rejects_non_bytes(schema_any): + """ + A native blob column stores raw bytes. Handing it an object silently stored + str(object) before the guard in __make_placeholder; it must raise instead. + """ + import numpy as np + + @schema_any + class RawBinary(dj.Manual): + definition = """ + id : int32 + --- + data : longblob + """ + + with pytest.raises(dj.DataJointError, match="native binary type"): + RawBinary.insert1({"id": 1, "data": np.arange(500, dtype="float32")}) + with pytest.raises(dj.DataJointError, match="native binary type"): + RawBinary.insert1({"id": 2, "data": [1, 2, 3]}) + with pytest.raises(dj.DataJointError, match="native binary type"): + RawBinary.insert1({"id": 3, "data": "text"}) + + # bytes are what the column is for, and must round-trip untouched + RawBinary.insert1({"id": 4, "data": b"\x00\x01\x02"}) + assert (RawBinary & "id=4").fetch1("data") == b"\x00\x01\x02" + assert len(RawBinary()) == 1 # none of the rejected inserts landed + RawBinary.drop_quick() + + +def test_bare_blob_datatype(schema_any): + """`blob` without a size prefix is a MySQL type in its own right.""" + + @schema_any + class BareBlob(dj.Manual): + definition = """ + id : int32 + --- + data : blob + """ + + BareBlob.insert1({"id": 1, "data": b"abc"}) + assert (BareBlob & "id=1").fetch1("data") == b"abc" + BareBlob.drop_quick() + + +def test_decimal_with_modifiers(schema_any): + """ + decimal(M,D) carrying `unsigned`, or spelled with a single argument, was valid in + 0.14.x and must remain declarable as a native type. + """ + from decimal import Decimal + + @schema_any + class Proportion(dj.Manual): + definition = """ + id : int32 + --- + on_proportion : decimal(2, 2) unsigned + scale : decimal(5) + """ + + Proportion.insert1({"id": 1, "on_proportion": Decimal("0.25"), "scale": 42}) + row = (Proportion & "id=1").fetch1() + assert row["on_proportion"] == Decimal("0.25") + assert int(row["scale"]) == 42 + # the modified decimal must be recognized as numeric, not fall through as unsupported + assert Proportion.heading["on_proportion"].numeric + Proportion.drop_quick() diff --git a/tests/unit/test_type_patterns.py b/tests/unit/test_type_patterns.py new file mode 100644 index 000000000..30396b48b --- /dev/null +++ b/tests/unit/test_type_patterns.py @@ -0,0 +1,160 @@ +""" +Type-pattern consistency and strictness. + +These tests cover the seam between the three places that name a type: the core type +table in ``declare``, the native passthrough patterns beside it, and the legacy mapping +in ``migrate``. Each of the bugs exercised here was a spelling that one of those three +accepted and another did not. +""" + +import pytest + +from datajoint.declare import CORE_TYPES, TYPE_PATTERN, match_type +from datajoint.errors import DataJointError +from datajoint.migrate import NATIVE_TO_CORE_TYPE + + +class TestCrossReferences: + """Every type named in one module must be recognized by the others.""" + + def test_migrate_core_types_are_declarable(self): + """ + Values in NATIVE_TO_CORE_TYPE are written into column comments as ``:type:`` + markers and read back by Heading. A value that match_type rejects produces a + marker that cannot be resolved on load. + """ + for native, core in NATIVE_TO_CORE_TYPE.items(): + if core.startswith("<"): + continue # codecs are resolved by the codec registry, not match_type + assert match_type(core), f"{native!r} maps to {core!r}, which match_type rejects" + + def test_migrate_native_types_are_recognized(self): + """The keys are types read from a legacy database and must classify too.""" + for native in NATIVE_TO_CORE_TYPE: + assert match_type(native) + + def test_core_type_names_are_declarable(self): + """Each core type's own canonical spelling must classify as that core type.""" + canonical = { + "float32": "float32", + "float64": "float64", + "int64": "int64", + "int32": "int32", + "int16": "int16", + "int8": "int8", + "bool": "bool", + "uuid": "uuid", + "json": "json", + "bytes": "bytes", + "date": "date", + "datetime": "datetime", + "char": "char(8)", + "varchar": "varchar(8)", + "enum": "enum('a','b')", + "decimal": "decimal(6,2)", + } + assert set(canonical) == set(CORE_TYPES), "CORE_TYPES changed; update this map" + for name, spelling in canonical.items(): + assert match_type(spelling) == name.upper() + + +class TestStrictness: + """A declared type must be consumed in its entirety, not merely prefixed.""" + + @pytest.mark.parametrize( + "spelling", + [ + "int24", # plausible-looking, and formerly emitted by migrate.py + "intbanana", + "integerish", + "tinyinteger", + "uint32", # unsigned core types are deliberately not provided + "uint8", + "float64x", + "completely_invalid_type_xyz", + ], + ) + def test_near_miss_spellings_are_rejected(self, spelling): + with pytest.raises(DataJointError, match="Unsupported attribute type"): + match_type(spelling) + + def test_serial_alternative_stays_anchored(self): + """`serial` is an alternative inside the INTEGER pattern; it must not leak.""" + assert match_type("serial") == "INTEGER" + with pytest.raises(DataJointError): + match_type("serialize") + + +class TestNumericAliases: + """decimal/numeric/dec/fixed are one SQL type and must be treated alike.""" + + def test_canonical_decimal_is_a_core_type(self): + assert match_type("decimal(6,2)") == "DECIMAL" + assert match_type("decimal(2, 2)") == "DECIMAL" + + @pytest.mark.parametrize( + "spelling", + [ + "decimal(2,2) unsigned", + "decimal(2, 2) unsigned", + "DECIMAL(2,2) UNSIGNED", + "decimal(2,2) zerofill", + "decimal(5)", + "decimal", + "dec(2,2)", + "fixed(2,2)", + "numeric(2,2)", + "numeric(2,2) unsigned", + ], + ) + def test_modified_and_aliased_forms_pass_as_native(self, spelling): + """ + These were valid in 0.14.x and must remain declarable. They pass as native + types with a warning rather than as the core decimal type. + """ + assert match_type(spelling) == "NUMERIC" + + def test_numeric_is_recognized_as_numeric_by_heading(self): + """ + Heading derives its `numeric` flag from the same patterns. A decimal carrying a + modifier must not fall through as neither numeric nor string. + """ + for spelling in ("decimal(2,2) unsigned", "numeric(6,2)", "decimal(5)"): + assert any(TYPE_PATTERN[t].match(spelling) for t in ("DECIMAL", "NUMERIC", "INTEGER", "FLOAT")) + + +class TestNativeSpellingsStillAccepted: + """Guard against the strictness change rejecting legitimate native types.""" + + @pytest.mark.parametrize( + "spelling,category", + [ + ("int", "INTEGER"), + ("int(11)", "INTEGER"), + ("int unsigned", "INTEGER"), + ("int(11) unsigned", "INTEGER"), + ("bigint unsigned auto_increment", "INTEGER"), + ("integer", "INTEGER"), + ("tinyint", "INTEGER"), + ("smallint", "INTEGER"), + ("double", "FLOAT"), + ("float", "FLOAT"), + ("real", "FLOAT"), + ("double unsigned", "FLOAT"), + ("varchar(255)", "VARCHAR"), + ("char(4)", "CHAR"), + ("timestamp", "TEMPORAL"), + ("time", "TEMPORAL"), + ("year", "TEMPORAL"), + ("longblob", "NATIVE_BLOB"), + ("mediumblob", "NATIVE_BLOB"), + ("blob", "NATIVE_BLOB"), # bare blob is a MySQL type too + ("longtext", "NATIVE_TEXT"), + ("text", "NATIVE_TEXT"), + ("", "CODEC"), + ("", "CODEC"), + ("", "CODEC"), + ], + ) + def test_spelling_classifies(self, spelling, category): + assert match_type(spelling) == category