From 0856912f4ea46b10dca75839fd9e27e22d0807e1 Mon Sep 17 00:00:00 2001 From: km Date: Sun, 16 Aug 2026 10:35:41 +0900 Subject: [PATCH] Warn when relationship to_columns does not cover a declared key The spec defines to_columns as "Primary/unique key columns in the 'to' dataset", but the validator only checked that relationship datasets exist. A relationship joining to non-key columns breaks many-to-one semantics downstream (see #301 for a converter emitting exactly this and consumers rejecting it). validate_references now checks that to_columns covers the to dataset's primary_key or one of its unique_keys. Coverage rather than exact equality: a superset of a key still guarantees the join cardinality, and the databricks converter's _covers_unique_key already applies the same semantics. Reported as a warning rather than an error because declared keys may be an incomplete recovery of the dataset's real keys, and datasets that declare no keys are skipped entirely. Shape guards keep the semantic check from crashing or misreporting on documents that already fail schema validation (null unique_keys, non-list to_columns, flat unique_keys). Co-Authored-By: Claude Fable 5 Signed-off-by: km --- validation/tests/test_validate.py | 163 ++++++++++++++++++++++++++++++ validation/validate.py | 26 ++++- 2 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 validation/tests/test_validate.py diff --git a/validation/tests/test_validate.py b/validation/tests/test_validate.py new file mode 100644 index 00000000..30806f18 --- /dev/null +++ b/validation/tests/test_validate.py @@ -0,0 +1,163 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from importlib.util import module_from_spec, spec_from_file_location +from pathlib import Path + +import pytest + +# validate.py exits at import time when its dependencies are missing, which +# would abort the whole pytest session during collection — skip instead. +pytest.importorskip("yaml") +pytest.importorskip("jsonschema") + +_VALIDATE_PATH = Path(__file__).parents[1] / "validate.py" +_SPEC = spec_from_file_location("ossie_validate", _VALIDATE_PATH) +assert _SPEC is not None and _SPEC.loader is not None +_VALIDATE = module_from_spec(_SPEC) +_SPEC.loader.exec_module(_VALIDATE) + +validate_references = _VALIDATE.validate_references + + +def _document(datasets: list[dict], relationships: list[dict]) -> dict: + return { + "version": "0.2.0.dev0", + "semantic_model": [ + { + "name": "m", + "datasets": datasets, + "relationships": relationships, + } + ], + } + + +_CUSTOMERS = { + "name": "customers", + "source": "db.s.customers", + "primary_key": ["id"], + "unique_keys": [["email"]], +} + +_ORDERS = {"name": "orders", "source": "db.s.orders"} + + +def _relationship(to_columns: list[str], to: str = "customers") -> dict: + return { + "name": "orders_to_customers", + "from": "orders", + "to": to, + "from_columns": ["customer_id"], + "to_columns": to_columns, + } + + +def test_warns_when_to_columns_does_not_cover_a_declared_key() -> None: + errors = validate_references( + _document([_ORDERS, _CUSTOMERS], [_relationship(to_columns=["region"])]) + ) + + assert errors == [ + "[Reference] Warning: Relationship 'orders_to_customers' in model 'm': " + "to_columns ['region'] does not cover the primary key or a unique key of dataset 'customers'" + ] + + +def test_accepts_to_columns_matching_the_primary_key() -> None: + errors = validate_references( + _document([_ORDERS, _CUSTOMERS], [_relationship(to_columns=["id"])]) + ) + + assert errors == [] + + +def test_accepts_to_columns_matching_a_unique_key() -> None: + errors = validate_references( + _document([_ORDERS, _CUSTOMERS], [_relationship(to_columns=["email"])]) + ) + + assert errors == [] + + +def test_accepts_to_columns_that_is_a_superset_of_a_key() -> None: + # e.g. tenant-sharded joins carry extra columns on top of the key; + # coverage still guarantees the many-to-one semantics. + errors = validate_references( + _document([_ORDERS, _CUSTOMERS], [_relationship(to_columns=["tenant_id", "id"])]) + ) + + assert errors == [] + + +def test_accepts_composite_key_regardless_of_column_order() -> None: + composite = { + "name": "order_lines", + "source": "db.s.order_lines", + "primary_key": ["order_id", "line_number"], + } + rel = _relationship(to_columns=["line_number", "order_id"], to="order_lines") + + assert validate_references(_document([_ORDERS, composite], [rel])) == [] + + +def test_skips_datasets_that_declare_no_keys() -> None: + no_keys = {"name": "raw_table", "source": "db.s.raw_table"} + rel = _relationship(to_columns=["anything"], to="raw_table") + + assert validate_references(_document([_ORDERS, no_keys], [rel])) == [] + + +def test_still_reports_unknown_datasets() -> None: + errors = validate_references( + _document([_ORDERS], [_relationship(to_columns=["id"], to="nope")]) + ) + + assert errors == [ + "[Reference] Relationship 'orders_to_customers' in model 'm' references unknown dataset 'nope'" + ] + + +def test_tolerates_null_unique_keys() -> None: + # `unique_keys:` present but empty parses to None; the check must not crash. + dataset = {"name": "customers", "source": "db.s.customers", + "primary_key": ["id"], "unique_keys": None} + errors = validate_references( + _document([_ORDERS, dataset], [_relationship(to_columns=["id"])]) + ) + + assert errors == [] + + +def test_skips_non_list_to_columns() -> None: + # Schema validation reports the shape error; the semantic check must + # neither crash nor emit a misleading character-set comparison. + rel = _relationship(to_columns=["id"]) + rel["to_columns"] = "id" + + assert validate_references(_document([_ORDERS, _CUSTOMERS], [rel])) == [] + + +def test_skips_malformed_flat_unique_keys() -> None: + # unique_keys mistakenly written flat like primary_key: strings are not + # keys, so with no well-formed key declared the check does not fire. + dataset = {"name": "customers", "source": "db.s.customers", "unique_keys": ["email"]} + errors = validate_references( + _document([_ORDERS, dataset], [_relationship(to_columns=["email"])]) + ) + + assert errors == [] diff --git a/validation/validate.py b/validation/validate.py index 258d34f1..88398c9c 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -129,23 +129,41 @@ def validate_unique_names(data: dict) -> list[str]: def validate_references(data: dict) -> list[str]: - """Validate that relationships reference existing datasets.""" + """Validate that relationships reference existing datasets and that + to_columns covers a declared key of the 'to' dataset.""" errors = [] for model in data.get("semantic_model", []): model_name = model.get("name", "") - dataset_names = {d.get("name") for d in model.get("datasets", []) if d.get("name")} + datasets = {d.get("name"): d for d in model.get("datasets", []) if d.get("name")} for rel in model.get("relationships", []): rel_name = rel.get("name", "") from_ds = rel.get("from") to_ds = rel.get("to") - if from_ds and from_ds not in dataset_names: + if from_ds and from_ds not in datasets: errors.append(f"[Reference] Relationship '{rel_name}' in model '{model_name}' references unknown dataset '{from_ds}'") - if to_ds and to_ds not in dataset_names: + if to_ds and to_ds not in datasets: errors.append(f"[Reference] Relationship '{rel_name}' in model '{model_name}' references unknown dataset '{to_ds}'") + # The spec defines to_columns as "Primary/unique key columns in the + # 'to' dataset". Coverage (superset of a key) still guarantees the + # many-to-one join, and declared keys may be incomplete since + # primary_key and unique_keys are optional — so accept any + # to_columns that covers a declared key, report a warning rather + # than an error, and skip datasets that declare no keys. + # Shape guards keep semantic checks from crashing on documents + # that already fail schema validation. + dataset = datasets.get(to_ds) + to_columns = rel.get("to_columns") + if dataset and isinstance(to_columns, list) and to_columns: + candidate_keys = [dataset.get("primary_key")] + list(dataset.get("unique_keys") or []) + declared_keys = [k for k in candidate_keys if isinstance(k, list) and k] + to_column_set = set(to_columns) + if declared_keys and not any(set(key) <= to_column_set for key in declared_keys): + errors.append(f"[Reference] Warning: Relationship '{rel_name}' in model '{model_name}': to_columns {to_columns} does not cover the primary key or a unique key of dataset '{to_ds}'") + return errors