From 4be804595edf633da78b827f74a37283f4d203bc Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Sat, 29 Aug 2026 10:20:13 +0200 Subject: [PATCH 1/4] fix(gooddata): preserve grain for aliased key fields Signed-off-by: Matt Faltyn --- .../src/ossie_gooddata/ossie_to_gooddata.py | 5 +-- .../gooddata/tests/test_ossie_to_gooddata.py | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py index 35c7be86..849b755c 100644 --- a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py +++ b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py @@ -162,13 +162,12 @@ def _convert_ossie_dataset( pk_columns = set(ds.get("primary_key", [])) for field_def in fields: - field_name = field_def["name"] is_dimension = field_def.get("dimension") is not None if is_dimension: attr = _convert_to_attribute(field_def, ds_name) attributes.append(attr) - if field_name in pk_columns: + if attr.source_column in pk_columns: grain_ids.append(attr.id) else: # Check MAQL expression to determine if fact or attribute @@ -176,7 +175,7 @@ def _convert_ossie_dataset( if maql_type == "attribute": attr = _convert_to_attribute(field_def, ds_name) attributes.append(attr) - if field_name in pk_columns: + if attr.source_column in pk_columns: grain_ids.append(attr.id) else: facts.append(_convert_to_fact(field_def, ds_name)) diff --git a/converters/gooddata/tests/test_ossie_to_gooddata.py b/converters/gooddata/tests/test_ossie_to_gooddata.py index 7422ffc0..63e3f5c1 100644 --- a/converters/gooddata/tests/test_ossie_to_gooddata.py +++ b/converters/gooddata/tests/test_ossie_to_gooddata.py @@ -330,6 +330,50 @@ def test_grain_from_primary_key(ossie_tpcds_dict: dict): assert len(grain_ids) == 2 +@pytest.mark.parametrize( + "field", + [ + { + "name": "customer_key", + "expression": {"dialects": [{"dialect": "ANSI_SQL", "expression": "customer_id"}]}, + "dimension": {}, + }, + { + "name": "customer_key", + "expression": { + "dialects": [ + {"dialect": "ANSI_SQL", "expression": "customer_id"}, + {"dialect": "MAQL", "expression": "{label/customers.customer_key}"}, + ] + }, + }, + ], + ids=["dimension", "maql-attribute"], +) +def test_grain_uses_source_column_for_aliased_attribute(field: dict): + """Verify physical primary keys select aliased GoodData grain attributes.""" + model = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + { + "name": "customers", + "source": "db.s.customers", + "primary_key": ["customer_id"], + "fields": [field], + } + ], + } + ] + } + + customer = ossie_to_gooddata(model).ldm.datasets[0] + + assert customer.attributes[0].source_column == "customer_id" + assert [grain.id for grain in customer.grain] == ["attr.customers.customer_key"] + + def test_relationships_become_references(ossie_tpcds_dict: dict): """Verify Ossie relationships become GoodData references.""" result = ossie_to_gooddata(ossie_tpcds_dict) From 55d0c0b0b160d23c749e19e6d4f06f2600f48ffb Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Sat, 12 Sep 2026 18:02:21 +0200 Subject: [PATCH 2/4] fix(gooddata): reject ambiguous source mappings Signed-off-by: Matt Faltyn --- .../src/ossie_gooddata/ossie_to_gooddata.py | 18 +++---- .../gooddata/tests/test_ossie_to_gooddata.py | 48 +++++++++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py index 849b755c..2b21f438 100644 --- a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py +++ b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py @@ -100,6 +100,10 @@ def _build_target_info(sm: dict[str, Any]) -> dict[str, dict[str, Any]]: if not is_date: for f in ds.get("fields", []): src = _get_source_column(f) + if src in col_to_attr: + raise ValueError( + f"Dataset '{ds_name}': source column '{src}' maps to multiple fields." + ) col_to_attr[src] = f"attr.{ds_name}.{f['name']}" info[ds_name] = {"is_date": is_date, "col_to_attr": col_to_attr} return info @@ -157,9 +161,6 @@ def _convert_ossie_dataset( # Regular dataset attributes: list[GdAttribute] = [] facts: list[GdFact] = [] - grain_ids: list[str] = [] - - pk_columns = set(ds.get("primary_key", [])) for field_def in fields: is_dimension = field_def.get("dimension") is not None @@ -167,20 +168,21 @@ def _convert_ossie_dataset( if is_dimension: attr = _convert_to_attribute(field_def, ds_name) attributes.append(attr) - if attr.source_column in pk_columns: - grain_ids.append(attr.id) else: # Check MAQL expression to determine if fact or attribute maql_type = _detect_type_from_maql(field_def) if maql_type == "attribute": attr = _convert_to_attribute(field_def, ds_name) attributes.append(attr) - if attr.source_column in pk_columns: - grain_ids.append(attr.id) else: facts.append(_convert_to_fact(field_def, ds_name)) - grain = [GdGrain(id=gid, type="attribute") for gid in grain_ids] + attribute_ids_by_column = {attr.source_column: attr.id for attr in attributes} + grain = [ + GdGrain(id=attribute_ids_by_column[column], type="attribute") + for column in ds.get("primary_key", []) + if column in attribute_ids_by_column + ] # Convert relationships from this dataset to GoodData references references = [] diff --git a/converters/gooddata/tests/test_ossie_to_gooddata.py b/converters/gooddata/tests/test_ossie_to_gooddata.py index 63e3f5c1..711668fb 100644 --- a/converters/gooddata/tests/test_ossie_to_gooddata.py +++ b/converters/gooddata/tests/test_ossie_to_gooddata.py @@ -374,6 +374,54 @@ def test_grain_uses_source_column_for_aliased_attribute(field: dict): assert [grain.id for grain in customer.grain] == ["attr.customers.customer_key"] +def test_duplicate_source_columns_are_rejected(): + """Verify ambiguous grain and relationship targets fail instead of being misassigned.""" + model = { + "semantic_model": [ + { + "name": "m", + "datasets": [ + { + "name": "customers", + "primary_key": ["customer_id"], + "fields": [ + _direct_field("customer_id", dimension={}), + { + "name": "customer_key", + "expression": { + "dialects": [ + {"dialect": "ANSI_SQL", "expression": "customer_id"} + ] + }, + "dimension": {}, + }, + ], + }, + { + "name": "orders", + "fields": [_direct_field("customer_id", dimension={})], + }, + ], + "relationships": [ + { + "name": "orders_customer", + "from": "orders", + "to": "customers", + "from_columns": ["customer_id"], + "to_columns": ["customer_id"], + } + ], + } + ] + } + + with pytest.raises( + ValueError, + match="Dataset 'customers': source column 'customer_id' maps to multiple fields", + ): + ossie_to_gooddata(model) + + def test_relationships_become_references(ossie_tpcds_dict: dict): """Verify Ossie relationships become GoodData references.""" result = ossie_to_gooddata(ossie_tpcds_dict) From 55e152af55b48953ae9facb718715f05fe466876 Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Mon, 28 Sep 2026 09:17:33 +0200 Subject: [PATCH 3/4] fix(gooddata): scope source mappings to attributes Signed-off-by: Matt Faltyn --- .../src/ossie_gooddata/ossie_to_gooddata.py | 4 +- .../gooddata/tests/test_ossie_to_gooddata.py | 115 +++++++++++------- 2 files changed, 75 insertions(+), 44 deletions(-) diff --git a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py index 2b21f438..35911c44 100644 --- a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py +++ b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py @@ -99,10 +99,12 @@ def _build_target_info(sm: dict[str, Any]) -> dict[str, dict[str, Any]]: col_to_attr: dict[str, str] = {} if not is_date: for f in ds.get("fields", []): + if f.get("dimension") is None and _detect_type_from_maql(f) != "attribute": + continue src = _get_source_column(f) if src in col_to_attr: raise ValueError( - f"Dataset '{ds_name}': source column '{src}' maps to multiple fields." + f"Dataset '{ds_name}': source column '{src}' maps to multiple attributes." ) col_to_attr[src] = f"attr.{ds_name}.{f['name']}" info[ds_name] = {"is_date": is_date, "col_to_attr": col_to_attr} diff --git a/converters/gooddata/tests/test_ossie_to_gooddata.py b/converters/gooddata/tests/test_ossie_to_gooddata.py index 711668fb..609fdbc9 100644 --- a/converters/gooddata/tests/test_ossie_to_gooddata.py +++ b/converters/gooddata/tests/test_ossie_to_gooddata.py @@ -353,19 +353,15 @@ def test_grain_from_primary_key(ossie_tpcds_dict: dict): def test_grain_uses_source_column_for_aliased_attribute(field: dict): """Verify physical primary keys select aliased GoodData grain attributes.""" model = { - "semantic_model": [ + "name": "m", + "datasets": [ { - "name": "m", - "datasets": [ - { - "name": "customers", - "source": "db.s.customers", - "primary_key": ["customer_id"], - "fields": [field], - } - ], + "name": "customers", + "source": "db.s.customers", + "primary_key": ["customer_id"], + "fields": [field], } - ] + ], } customer = ossie_to_gooddata(model).ldm.datasets[0] @@ -377,51 +373,84 @@ def test_grain_uses_source_column_for_aliased_attribute(field: dict): def test_duplicate_source_columns_are_rejected(): """Verify ambiguous grain and relationship targets fail instead of being misassigned.""" model = { - "semantic_model": [ + "name": "m", + "datasets": [ { - "name": "m", - "datasets": [ - { - "name": "customers", - "primary_key": ["customer_id"], - "fields": [ - _direct_field("customer_id", dimension={}), - { - "name": "customer_key", - "expression": { - "dialects": [ - {"dialect": "ANSI_SQL", "expression": "customer_id"} - ] - }, - "dimension": {}, - }, - ], - }, + "name": "customers", + "primary_key": ["customer_id"], + "fields": [ + _direct_field("customer_id", dimension={}), { - "name": "orders", - "fields": [_direct_field("customer_id", dimension={})], + "name": "customer_key", + "expression": { + "dialects": [{"dialect": "ANSI_SQL", "expression": "customer_id"}] + }, + "dimension": {}, }, ], - "relationships": [ - { - "name": "orders_customer", - "from": "orders", - "to": "customers", - "from_columns": ["customer_id"], - "to_columns": ["customer_id"], - } - ], + }, + { + "name": "orders", + "fields": [_direct_field("customer_id", dimension={})], + }, + ], + "relationships": [ + { + "name": "orders_customer", + "from": "orders", + "to": "customers", + "from_columns": ["customer_id"], + "to_columns": ["customer_id"], } - ] + ], } with pytest.raises( ValueError, - match="Dataset 'customers': source column 'customer_id' maps to multiple fields", + match="Dataset 'customers': source column 'customer_id' maps to multiple attributes", ): ossie_to_gooddata(model) +def test_attribute_and_facts_sharing_source_column_are_permitted(): + """Facts sharing a source column do not make grain or reference targets ambiguous.""" + fact = _direct_field("customer_id") + model = { + "name": "m", + "datasets": [ + { + "name": "customers", + "primary_key": ["customer_id"], + "fields": [ + _direct_field("customer_id", dimension={}), + {**fact, "name": "customer_count"}, + {**fact, "name": "customer_total"}, + ], + }, + { + "name": "orders", + "fields": [_direct_field("customer_id", dimension={})], + }, + ], + "relationships": [ + { + "name": "orders_customer", + "from": "orders", + "to": "customers", + "from_columns": ["customer_id"], + "to_columns": ["customer_id"], + } + ], + } + + customers, orders = ossie_to_gooddata(model).ldm.datasets + + assert len(customers.attributes) == 1 + assert len(customers.facts) == 2 + assert [grain.id for grain in customers.grain] == ["attr.customers.customer_id"] + assert orders.references[0].sources[0].target.id == "attr.customers.customer_id" + + def test_relationships_become_references(ossie_tpcds_dict: dict): """Verify Ossie relationships become GoodData references.""" result = ossie_to_gooddata(ossie_tpcds_dict) From a685310031def10a35af2af9fcdff86f5444eb3d Mon Sep 17 00:00:00 2001 From: Matt Faltyn Date: Mon, 28 Sep 2026 09:25:14 +0200 Subject: [PATCH 4/4] fix(gooddata): deduplicate grain for repeated key columns Signed-off-by: Matt Faltyn --- .../gooddata/src/ossie_gooddata/ossie_to_gooddata.py | 2 +- converters/gooddata/tests/test_ossie_to_gooddata.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py index 35911c44..49c8e786 100644 --- a/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py +++ b/converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py @@ -182,7 +182,7 @@ def _convert_ossie_dataset( attribute_ids_by_column = {attr.source_column: attr.id for attr in attributes} grain = [ GdGrain(id=attribute_ids_by_column[column], type="attribute") - for column in ds.get("primary_key", []) + for column in dict.fromkeys(ds.get("primary_key", [])) if column in attribute_ids_by_column ] diff --git a/converters/gooddata/tests/test_ossie_to_gooddata.py b/converters/gooddata/tests/test_ossie_to_gooddata.py index 609fdbc9..4679c229 100644 --- a/converters/gooddata/tests/test_ossie_to_gooddata.py +++ b/converters/gooddata/tests/test_ossie_to_gooddata.py @@ -330,6 +330,11 @@ def test_grain_from_primary_key(ossie_tpcds_dict: dict): assert len(grain_ids) == 2 +@pytest.mark.parametrize( + "primary_key", + [["customer_id"], ["customer_id", "customer_id"]], + ids=["single-key", "repeated-key"], +) @pytest.mark.parametrize( "field", [ @@ -350,7 +355,7 @@ def test_grain_from_primary_key(ossie_tpcds_dict: dict): ], ids=["dimension", "maql-attribute"], ) -def test_grain_uses_source_column_for_aliased_attribute(field: dict): +def test_grain_uses_source_column_for_aliased_attribute(field: dict, primary_key: list[str]): """Verify physical primary keys select aliased GoodData grain attributes.""" model = { "name": "m", @@ -358,7 +363,7 @@ def test_grain_uses_source_column_for_aliased_attribute(field: dict): { "name": "customers", "source": "db.s.customers", - "primary_key": ["customer_id"], + "primary_key": primary_key, "fields": [field], } ],