diff --git a/CHANGELOG.md b/CHANGELOG.md index 447e9cf64..54c7b3052 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `datacontract test --dry-run` reports the checks a run would execute without connecting to the server or reading any data (#1510) ### Fixed +- `datacontract export jsonschema`, `datacontract export avro` and `datacontract test` on local files use a property's `physicalName` as the field name when set, instead of the logical `name` (#1494) - `datacontract import sql` takes the server's `database` and `schema` from a qualified `CREATE TABLE`, instead of always writing placeholders (#651) - `datacontract import sql` no longer fails on a DDL file that contains `CREATE SCHEMA` (#1529) - `datacontract test` now supports ISO 8601 retention periods correctly (previously, only the first component was considered) (#1538) diff --git a/datacontract/engines/ibis/connections/duckdb_connection.py b/datacontract/engines/ibis/connections/duckdb_connection.py index f439780a6..c0e455bbe 100644 --- a/datacontract/engines/ibis/connections/duckdb_connection.py +++ b/datacontract/engines/ibis/connections/duckdb_connection.py @@ -215,7 +215,7 @@ def to_csv_types(schema_obj: SchemaObject) -> dict[Any, str | None] | None: columns = {} if schema_obj.properties: for prop in schema_obj.properties: - columns[prop.name] = convert_to_duckdb_csv_type(prop) + columns[prop.physicalName or prop.name] = convert_to_duckdb_csv_type(prop) return columns @@ -226,7 +226,7 @@ def to_parquet_types(schema_obj: SchemaObject) -> dict[Any, str | None] | None: columns = {} if schema_obj.properties: for prop in schema_obj.properties: - columns[prop.name] = convert_to_duckdb(prop) + columns[prop.physicalName or prop.name] = convert_to_duckdb(prop) return columns @@ -236,7 +236,7 @@ def to_json_types(schema_obj: SchemaObject) -> dict[Any, str | None] | None: columns = {} if schema_obj.properties: for prop in schema_obj.properties: - columns[prop.name] = convert_to_duckdb_json_type(prop) + columns[prop.physicalName or prop.name] = convert_to_duckdb_json_type(prop) return columns @@ -263,16 +263,17 @@ def add_nested_views(con: "duckdb.DuckDBPyConnection", model_name: str, properti elif field_type == "object" and (prop.properties is None or len(prop.properties) == 0): continue - nested_model_name = f"{model_name}__{prop.name}" + field_name = prop.physicalName or prop.name + nested_model_name = f"{model_name}__{field_name}" max_depth = 2 if field_type == "array" else 1 ## if parent field is not required, the nested objects may resolve ## to a row of NULLs -- but if the objects themselves have required ## fields, this will fail the check. - where = "" if prop.required else f" WHERE {prop.name} IS NOT NULL" + where = "" if prop.required else f" WHERE {field_name} IS NOT NULL" con.sql(f""" CREATE VIEW IF NOT EXISTS "{nested_model_name}" AS - SELECT unnest({prop.name}, max_depth := {max_depth}) as {prop.name} FROM "{model_name}" {where} + SELECT unnest({field_name}, max_depth := {max_depth}) as {field_name} FROM "{model_name}" {where} """) if field_type == "array": add_nested_views(con, nested_model_name, prop.items.properties if prop.items else None) diff --git a/datacontract/export/avro_exporter.py b/datacontract/export/avro_exporter.py index 78a358cd0..0182c757e 100644 --- a/datacontract/export/avro_exporter.py +++ b/datacontract/export/avro_exporter.py @@ -106,7 +106,7 @@ def _parse_default_value(value: str): def to_avro_field(prop: SchemaProperty) -> dict: - avro_field = {"name": prop.name} + avro_field = {"name": prop.physicalName or prop.name} if prop.description is not None: avro_field["doc"] = prop.description is_required_avro = prop.required if prop.required is not None else True diff --git a/datacontract/export/duckdb_type_converter.py b/datacontract/export/duckdb_type_converter.py index 2f1e6c825..a266c6ead 100644 --- a/datacontract/export/duckdb_type_converter.py +++ b/datacontract/export/duckdb_type_converter.py @@ -66,5 +66,5 @@ def convert_to_duckdb_json_type(prop: SchemaProperty) -> None | str: def convert_to_duckdb_object(properties: List[SchemaProperty]): - columns = [f'"{prop.name}" {convert_to_duckdb_json_type(prop)}' for prop in properties] + columns = [f'"{prop.physicalName or prop.name}" {convert_to_duckdb_json_type(prop)}' for prop in properties] return f"STRUCT({', '.join(columns)})" diff --git a/datacontract/export/jsonschema_exporter.py b/datacontract/export/jsonschema_exporter.py index cf2f9296b..2810c2fc6 100644 --- a/datacontract/export/jsonschema_exporter.py +++ b/datacontract/export/jsonschema_exporter.py @@ -29,7 +29,7 @@ def to_jsonschema_json(model_key: str, model_value: SchemaObject) -> str: def to_properties(properties: List[SchemaProperty]) -> dict: result = {} for prop in properties: - result[prop.name] = to_property(prop) + result[prop.physicalName or prop.name] = to_property(prop) return result @@ -94,7 +94,7 @@ def to_property(prop: SchemaProperty) -> dict: if json_type == "object": nested_props = prop.properties or [] # TODO: any better idea to distinguish between properties and patternProperties? - if nested_props and nested_props[0].name.startswith("^"): + if nested_props and (nested_props[0].physicalName or nested_props[0].name).startswith("^"): property_dict["patternProperties"] = to_properties(nested_props) else: property_dict["properties"] = to_properties(nested_props) @@ -171,7 +171,7 @@ def to_required(properties: List[SchemaProperty]) -> list: required = [] for prop in properties: if prop.required is True: - required.append(prop.name) + required.append(prop.physicalName or prop.name) return required diff --git a/tests/test_duckdb_json.py b/tests/test_duckdb_json.py index 65e342093..85f6c0221 100644 --- a/tests/test_duckdb_json.py +++ b/tests/test_duckdb_json.py @@ -145,3 +145,51 @@ def test_empty_object(): assert "sample_data" in table_names assert "sample_data__metadata" not in table_names assert "sample_data__settings" not in table_names + + +def test_physical_name_columns(): + """Columns come from physicalName when it is set, so the checks find them.""" + data_contract_str = """ +kind: DataContract +apiVersion: v3.1.0 +id: "61111-0003" +name: Sample data with physical names +version: 1.0.0 +status: active +servers: + - server: sample + type: local + path: ./fixtures/local-json/data/nested_types.json + format: json + delimiter: array +schema: + - name: sample_data + physicalType: object + properties: + - name: Identifier + physicalName: id + logicalType: integer + - name: Tag List + physicalName: tags + logicalType: array + items: + logicalType: object + properties: + - name: Foo Value + physicalName: foo + logicalType: string + - name: Full Name + physicalName: name + logicalType: object + properties: + - name: First Name + physicalName: first + logicalType: string + """ + data_contract = resolve.resolve_data_contract(data_contract_str=data_contract_str) + server = next(s for s in data_contract.servers if s.server == "sample") + con = get_duckdb_connection(data_contract, server, Run.create_run()) + assert con.table("sample_data").columns == ["id", "tags", "name"] + # nested struct fields and the nested views are keyed on the physical name too + assert con.table("sample_data__tags").columns == ["foo"] + assert con.table("sample_data__name").columns == ["first"] diff --git a/tests/test_export_avro.py b/tests/test_export_avro.py index e9b26434e..b901af2a9 100644 --- a/tests/test_export_avro.py +++ b/tests/test_export_avro.py @@ -1,6 +1,7 @@ import json from datacontract_specification.model import DataContractSpecification +from open_data_contract_standard.model import SchemaObject, SchemaProperty from typer.testing import CliRunner from datacontract.cli import app @@ -100,6 +101,22 @@ def test_to_field_map(): assert json.loads(result) == json.loads(expected_avro_schema) +def test_to_avro_schema_uses_physical_name(): + schema = SchemaObject( + name="equipment", + properties=[ + SchemaProperty(name="Equipment Name", physicalName="name", logicalType="string", required=True), + SchemaProperty(name="location", logicalType="string", required=False), + ], + ) + + result = json.loads(to_avro_schema_json("equipment", schema)) + + # physicalName wins over the logical name; falls back to name when unset + field_names = [field["name"] for field in result["fields"]] + assert field_names == ["name", "location"] + + def test_to_field_float(): dcs = DataContractSpecification.from_file("fixtures/avro/export/datacontract_test_field_float.yaml") data_contract = convert_dcs_to_odcs(dcs) diff --git a/tests/test_export_jsonschema.py b/tests/test_export_jsonschema.py index 59e17d235..755cfc1fb 100644 --- a/tests/test_export_jsonschema.py +++ b/tests/test_export_jsonschema.py @@ -2,11 +2,12 @@ import os import sys +from open_data_contract_standard.model import SchemaObject, SchemaProperty from typer.testing import CliRunner from datacontract.cli import app from datacontract.data_contract import DataContract -from datacontract.export.jsonschema_exporter import to_jsonschemas +from datacontract.export.jsonschema_exporter import to_jsonschema, to_jsonschemas from datacontract.lint.resolve import resolve_data_contract # logging.basicConfig(level=logging.DEBUG, force=True) @@ -175,6 +176,23 @@ def test_to_jsonschemas_complex_2(): assert result["sts_data"] == json.loads(expected_json_schema) +def test_to_jsonschema_uses_physical_name(): + schema = SchemaObject( + name="equipment", + properties=[ + SchemaProperty(name="Equipment Name", physicalName="name", logicalType="string", required=True), + SchemaProperty(name="location", logicalType="string", required=False), + ], + ) + + result = to_jsonschema("equipment", schema) + + # physicalName wins over the logical name; falls back to name when unset + assert set(result["properties"].keys()) == {"name", "location"} + assert result["properties"]["name"]["type"] == "string" + assert result["required"] == ["name"] + + def read_file(data_contract_file): if not os.path.exists(data_contract_file): print(f"The file '{data_contract_file}' does not exist.")