diff --git a/core-spec/ossie-schema.json b/core-spec/ossie-schema.json index 7b9c9991f..a8d2fcbdc 100644 --- a/core-spec/ossie-schema.json +++ b/core-spec/ossie-schema.json @@ -155,6 +155,7 @@ "properties": { "name": { "type": "string", + "minLength": 1, "description": "Unique identifier for the field within the dataset" }, "expression": { @@ -193,10 +194,12 @@ "properties": { "name": { "type": "string", + "minLength": 1, "description": "Unique identifier for the dataset" }, "source": { "type": "string", + "minLength": 1, "description": "Reference to underlying physical table/view (database.schema.table) or query" }, "primary_key": { @@ -245,14 +248,17 @@ "properties": { "name": { "type": "string", + "minLength": 1, "description": "Unique identifier for the relationship" }, "from": { "type": "string", + "minLength": 1, "description": "Dataset on the many side of the relationship" }, "to": { "type": "string", + "minLength": 1, "description": "Dataset on the one side of the relationship" }, "from_columns": { @@ -290,6 +296,7 @@ "properties": { "name": { "type": "string", + "minLength": 1, "description": "Unique identifier for the metric" }, "expression": { @@ -321,6 +328,7 @@ "properties": { "name": { "type": "string", + "minLength": 1, "description": "Unique identifier for the semantic model" }, "description": { diff --git a/validation/test_validate.py b/validation/test_validate.py index 2519b9550..bf1b2c2b6 100644 --- a/validation/test_validate.py +++ b/validation/test_validate.py @@ -420,6 +420,39 @@ def test_root_dialects_and_vendors_are_rejected(self): self.assertEqual(result.returncode, 1) self.assertIn("'dialects', 'vendors' were unexpected", result.stdout) + def test_deeply_nested_yaml_fails_cleanly(self): + # Deeply nested flow collections exhaust the recursion limit during + # composition, surfacing as RecursionError rather than YAMLError. The + # validator must exit with a diagnostic, never a raw traceback. + result = self.run_validator("[" * 3000 + "]" * 3000 + "\n") + + self.assertEqual(result.returncode, 1) + self.assertNotIn("Traceback", result.stderr) + self.assertIn("too deeply nested", result.stdout) + + def test_empty_relationship_endpoint_is_rejected_by_schema(self): + # An empty relationship endpoint ("") is invalid data; the schema + # (minLength: 1) rejects it end-to-end before semantic checks run, + # rather than the model silently passing. + result = self.run_validator( + "version: 0.2.0.dev0\n" + "name: sales\n" + "datasets:\n" + " - name: orders\n" + " source: analytics.orders\n" + "relationships:\n" + " - name: orders_to_missing\n" + " from: orders\n" + " to: ''\n" + " from_columns: [customer_id]\n" + " to_columns: [id]\n" + ) + + self.assertEqual(result.returncode, 1) + self.assertNotIn("Traceback", result.stderr) + self.assertIn("Validation FAILED", result.stdout) + self.assertIn("[Schema] relationships -> 0 -> to: '' should be non-empty", result.stdout) + if __name__ == "__main__": unittest.main() diff --git a/validation/tests/test_validate.py b/validation/tests/test_validate.py index 8583c53c7..22a86735f 100644 --- a/validation/tests/test_validate.py +++ b/validation/tests/test_validate.py @@ -166,6 +166,35 @@ def test_unique_names_are_checked_in_the_root_model() -> None: assert errors == ["[Unique] Duplicate dataset name 'orders' in model 'm'"] +@pytest.mark.parametrize( + ("mutate", "path"), + [ + (lambda doc: doc.update(name=""), "name"), + (lambda doc: doc["datasets"][0].update(name=""), "datasets -> 0 -> name"), + (lambda doc: doc["datasets"][0].update(source=""), "datasets -> 0 -> source"), + ], +) +def test_schema_rejects_empty_identifiers(core_schema: dict, mutate, path: str) -> None: + # Empty identifiers are invalid data. The schema (minLength: 1) rejects them + # up front, so no downstream check has to special-case "" -- and a single + # empty name is caught even though it never trips duplicate detection. + document = _document([dict(_ORDERS)], []) + mutate(document) + + assert _VALIDATE.validate_schema(document, core_schema) == [ + f"[Schema] {path}: '' should be non-empty" + ] + + +def test_schema_rejects_empty_relationship_endpoints(core_schema: dict) -> None: + document = _document([dict(_ORDERS), dict(_CUSTOMERS)], [_relationship(to_columns=["id"])]) + document["relationships"][0]["to"] = "" + + assert _VALIDATE.validate_schema(document, core_schema) == [ + "[Schema] relationships -> 0 -> to: '' should be non-empty" + ] + + def test_sql_checks_traverse_root_fields_and_metrics(monkeypatch: pytest.MonkeyPatch) -> None: seen = [] @@ -261,6 +290,26 @@ def test_still_reports_unknown_datasets() -> None: ] +@pytest.mark.skipif(not _VALIDATE.SQLGLOT_AVAILABLE, reason="sqlglot is not installed") +def test_deeply_nested_sql_reports_a_diagnostic_instead_of_crashing() -> None: + # Pathologically nested SQL exhausts sqlglot's recursion limit; the + # validator must turn that into a diagnostic rather than propagating + # RecursionError and aborting the run. + expression = "(" * 5000 + "1" + ")" * 5000 + result = _VALIDATE.validate_sql_expression(expression, "ANSI_SQL", "ctx") + + assert result == "[SQL] ctx: expression is too deeply nested to parse" + + +@pytest.mark.skipif(not _VALIDATE.SQLGLOT_AVAILABLE, reason="sqlglot is not installed") +def test_non_string_expression_surfaces_instead_of_being_reported_valid() -> None: + # Only RecursionError is caught, so a genuine bug -- a non-string expression + # reaching the parser -- raises loudly rather than being masked as valid SQL + # by the SELECT-wrapped retry. + with pytest.raises(TypeError): + _VALIDATE.validate_sql_expression(123, "ANSI_SQL", "ctx") + + 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", diff --git a/validation/validate.py b/validation/validate.py index 99b390c6f..66548f58f 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -292,7 +292,11 @@ def validate_sql_expression(expr: str, dialect: str, context: str) -> str | None # Try parsing as expression first (for field expressions like "column_name") sqlglot.parse_one(expr, dialect=sqlglot_dialect) return None - except (ParseError, TokenError): + except (ParseError, TokenError, RecursionError): + # A bare column reference fails to parse alone; retry it wrapped in + # SELECT below. RecursionError (deeply nested input) is included so the + # retry reports it instead of crashing, while genuine errors such as a + # non-string expr raising TypeError still surface. pass try: @@ -301,6 +305,10 @@ def validate_sql_expression(expr: str, dialect: str, context: str) -> str | None return None except (ParseError, TokenError) as e: return f"[SQL] {context}: {str(e).split(chr(10))[0]}" + except RecursionError: + # Deeply nested input exhausts the recursion limit rather than raising a + # parser error; report it instead of letting it abort validation. + return f"[SQL] {context}: expression is too deeply nested to parse" def validate_sql(data: dict) -> list[str]: @@ -384,6 +392,10 @@ def main(): except yaml.YAMLError as e: print(f"Error: Invalid YAML: {e}") sys.exit(1) + except RecursionError: + # Deeply nested input surfaces as RecursionError, not YAMLError. + print("Error: Invalid YAML: input is too deeply nested to parse") + sys.exit(1) # Run validations errors = []