diff --git a/src/datajoint/condition.py b/src/datajoint/condition.py index f5789348a..650015539 100644 --- a/src/datajoint/condition.py +++ b/src/datajoint/condition.py @@ -218,7 +218,9 @@ def assert_join_compatibility( Second expression. semantic_check : bool, optional If True (default), use semantic matching and error on non-homologous - namesakes (same name, different lineage). If False, use natural join. + namesakes: the same name with different lineages, or with no lineage on + either side because no foreign key brings the attribute in. If False, + use natural join. Raises ------ @@ -261,32 +263,32 @@ def assert_join_compatibility( ) return - # Error on non-homologous namesakes + # Error on non-homologous namesakes. Lineage comes from foreign keys: an + # attribute that no foreign key brings in (a native secondary attribute, + # one computed in a projection, or a key that dj.U promotes) has none, and + # two such namesakes are unrelated, whether or not they are in a key. namesakes = set(expr1.heading.names) & set(expr2.heading.names) - for name in namesakes: + for name in sorted(namesakes): lineage1 = expr1.heading[name].lineage lineage2 = expr2.heading[name].lineage - # Semantic match requires both lineages to be non-None and equal - if lineage1 is None or lineage2 is None or lineage1 != lineage2: - if lineage1 is None or lineage2 is None: - # Missing lineage usually means stale ~lineage rows that survived - # an upgrade or a partial declare. Decoration in 2.3+ refreshes - # lineage automatically, so this typically indicates a schema - # that has not been re-decorated since the upgrade. - raise DataJointError( - f"Cannot join on attribute `{name}`: lineage missing on " - f"one side ({lineage1} vs {lineage2}). This usually " - f"indicates a stale `~lineage` entry from an older " - f"DataJoint version or an incomplete declare. Run " - f"`schema.rebuild_lineage()` to recompute lineage from " - f"current FK definitions. If the lineages are genuinely " - f"different, use `.proj()` to rename one of the attributes." - ) + if lineage1 is not None and lineage1 == lineage2: + continue + if lineage1 is None and lineage2 is None: raise DataJointError( - f"Cannot join on attribute `{name}`: " - f"different lineages ({lineage1} vs {lineage2}). " - f"Use .proj() to rename one of the attributes." + f"Cannot join on attribute `{name}`: neither side inherits it through a foreign key, " + f"so the two are unrelated. Use .proj() to rename or exclude it on one side." ) + if lineage1 is None or lineage2 is None: + raise DataJointError( + f"Cannot join on attribute `{name}`: one side traces it to {lineage1 or lineage2}, " + f"but the other does not inherit it through a foreign key, so the two are unrelated. " + f"Use .proj() to rename or exclude it on one side." + ) + raise DataJointError( + f"Cannot join on attribute `{name}`: " + f"different lineages ({lineage1} vs {lineage2}). " + f"Use .proj() to rename one of the attributes." + ) def make_condition( diff --git a/tests/integration/test_join_namesakes.py b/tests/integration/test_join_namesakes.py new file mode 100644 index 000000000..fa2ab3912 --- /dev/null +++ b/tests/integration/test_join_namesakes.py @@ -0,0 +1,114 @@ +""" +Joins on namesake attributes that no foreign key relates (#1640). + +Lineage comes from foreign keys. An attribute that no foreign key brings in, such +as a native secondary attribute, a computed attribute or a key promoted by +``dj.U``, has no lineage, and a join on two such namesakes is refused as a join +on unrelated attributes, not as missing lineage. +""" + +import time + +import pytest + +import datajoint as dj +from datajoint.errors import DataJointError + + +@pytest.fixture(scope="function") +def schema_by_backend(connection_by_backend, db_creds_by_backend): + backend = db_creds_by_backend["backend"] + schema_name = f"djtest_namesakes_{backend}_{str(int(time.time() * 1000))[-8:]}" + schema = dj.Schema(schema_name, connection=connection_by_backend) + yield schema + schema.drop(prompt=False) + + +@pytest.fixture +def tables(schema_by_backend): + @schema_by_backend + class Image(dj.Manual): + definition = """ + image_id : int32 + """ + + @schema_by_backend + class Segmentation(dj.Manual): + definition = """ + -> Image + --- + input_hash : varchar(64) + cell_count : int32 + """ + + @schema_by_backend + class Profile(dj.Manual): + definition = """ + -> Image + --- + input_hash : varchar(64) + mean_area : float32 + """ + + @schema_by_backend + class Label(dj.Manual): + definition = """ + label_id : int32 + --- + image_id : int32 # a plain number, not a reference to Image + """ + + Image.insert([(1,), (2,)]) + Segmentation.insert([(1, "a", 10), (2, "b", 20)]) + Profile.insert([(1, "a", 1.5), (2, "c", 2.5)]) + Label.insert([(1, 1)]) + return Image, Segmentation, Profile, Label + + +UNRELATED = "neither side inherits it through a foreign key" + + +def test_secondary_namesakes(schema_by_backend, db_creds_by_backend, tables): + """Namesake secondary attributes of two tables: refused as unrelated, with no + suggestion to rebuild lineage, and the same after rebuilding it.""" + _, Segmentation, Profile, _ = tables + with pytest.raises(DataJointError, match=UNRELATED) as exc_info: + Segmentation * Profile + assert "`input_hash`" in str(exc_info.value) + assert "rebuild_lineage" not in str(exc_info.value) + + if db_creds_by_backend["backend"] == "mysql": # rebuild_lineage() is MySQL-only for now + schema_by_backend.rebuild_lineage() + with pytest.raises(DataJointError, match=UNRELATED): + Segmentation * Profile + + +def test_computed_namesakes(tables): + """Computed attributes of two projections are unrelated namesakes.""" + _, Segmentation, Profile, _ = tables + with pytest.raises(DataJointError, match=UNRELATED): + Segmentation.proj(n="cell_count + 1") * Profile.proj(n="mean_area * 2") + + +def test_promoted_key_namesakes(tables): + """Primary-key attributes can lack lineage too: keys promoted by dj.U.""" + _, Segmentation, Profile, _ = tables + left = dj.U("input_hash") & Segmentation + right = dj.U("input_hash") & Profile + assert "input_hash" in left.primary_key + with pytest.raises(DataJointError, match=UNRELATED): + left * right + + +def test_one_sided_namesake(tables): + """A native key against a secondary namesake that no foreign key brings in.""" + Image, _, _, Label = tables + with pytest.raises(DataJointError, match="but the other does not inherit it through a foreign key"): + Image * Label + + +def test_renamed_or_excluded_namesake_joins(tables): + """The remedy the error names: rename or exclude the attribute on one side.""" + _, Segmentation, Profile, _ = tables + assert len(Segmentation * Profile.proj("mean_area")) == 2 + assert len(Segmentation * Profile.proj("mean_area", profile_hash="input_hash")) == 2 diff --git a/tests/integration/test_semantic_matching.py b/tests/integration/test_semantic_matching.py index 3d468a2b7..b51460921 100644 --- a/tests/integration/test_semantic_matching.py +++ b/tests/integration/test_semantic_matching.py @@ -466,10 +466,12 @@ def test_stale_non_none_lineage_not_auto_refreshed(self, schema_semantic): schema_semantic.rebuild_lineage() Trial._heading = Heading(table_info=old_heading.table_info) - def test_missing_lineage_error_points_to_rebuild(self, schema_semantic): + def test_missing_lineage_is_an_unrelated_namesake(self, schema_semantic): """ - When a join fails because one side has None lineage, the error must - point the user at `schema.rebuild_lineage()`. + A join on an attribute with no lineage on one side is refused as a join + on unrelated namesakes. The error does not send the user to rebuild + lineage: an attribute without lineage is usually one that no foreign key + brings in, which a rebuild cannot change (#1640). """ from datajoint.lineage import delete_table_lineages from datajoint.heading import Heading @@ -482,10 +484,9 @@ def test_missing_lineage_error_points_to_rebuild(self, schema_semantic): try: assert Enrollment().heading["student_id"].lineage is None - with pytest.raises(DataJointError) as exc_info: + with pytest.raises(DataJointError, match="does not inherit it through a foreign key") as exc_info: Student() * Enrollment() - assert "rebuild_lineage" in str(exc_info.value), f"Error must mention rebuild_lineage(); got: {exc_info.value}" - assert "stale" in str(exc_info.value).lower() or "missing" in str(exc_info.value).lower() + assert "rebuild_lineage" not in str(exc_info.value) finally: # Restore lineage so subsequent tests see clean state schema_semantic.rebuild_lineage()