Skip to content

Commit c9c54c8

Browse files
authored
Add dunder prefix to column aliases for simple-metric inputs (#1938)
To avoid alias conflicts in the generated SQL when a semantic model has a dimension and a simple metric with the same name, this PR updates the naming scheme for SQL column aliases. The longer term fix is to add a validation so that semantic models are required to have unique metric / measure / entity names. Due to the large snapshot changes, please view by commit.
1 parent cac02cd commit c9c54c8

File tree

2,523 files changed

+81584
-81259
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

2,523 files changed

+81584
-81259
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: Fixes
2+
body: Add dunder prefix to column aliases for simple-metric inputs
3+
time: 2025-11-17T08:54:11.838469-08:00
4+
custom:
5+
Author: plypaul
6+
Issue: "1938"

metricflow-semantics/metricflow_semantics/specs/dunder_column_association_resolver.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ def resolve_spec(self, spec: InstanceSpec) -> ColumnAssociation: # noqa: D102
3535

3636

3737
class DunderColumnAssociationResolverVisitor(InstanceSpecVisitor[ColumnAssociation]):
38-
"""Visitor helper class for DefaultColumnAssociationResolver2."""
38+
"""Visitor helper class for DefaultColumnAssociationResolver."""
3939

4040
def visit_metric_spec(self, metric_spec: MetricSpec) -> ColumnAssociation: # noqa: D102
4141
return ColumnAssociation(metric_spec.alias or metric_spec.element_name)
4242

4343
def visit_simple_metric_input_spec(self, spec: SimpleMetricInputSpec) -> ColumnAssociation: # noqa: D102
44-
return ColumnAssociation(spec.element_name)
44+
return ColumnAssociation(DUNDER + spec.element_name)
4545

4646
def visit_dimension_spec(self, dimension_spec: DimensionSpec) -> ColumnAssociation: # noqa: D102
4747
return ColumnAssociation(dimension_spec.alias or dimension_spec.dunder_name)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
from __future__ import annotations
2+
3+
from metricflow_semantics.test_helpers.config_helpers import DirectoryPathAnchor
4+
5+
NAME_EDGE_CASE_MANIFEST_ANCHOR = DirectoryPathAnchor()
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
metric:
3+
name: conflicted_name
4+
description: |
5+
Used to test a soon-to-be-deprecated use case where the name of the metric is the same as a dimension.
6+
type: simple
7+
type_params:
8+
measure: booking_value
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../shared/project_configuration.yaml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
semantic_model:
3+
name: bookings_source
4+
description: bookings_source
5+
6+
node_relation:
7+
schema_name: $source_schema
8+
alias: fct_bookings
9+
10+
defaults:
11+
agg_time_dimension: ds
12+
13+
measures:
14+
- name: booking_value
15+
agg: sum
16+
17+
dimensions:
18+
- name: conflicted_name
19+
expr: "'dummy_dimension_value'"
20+
type: categorical
21+
- name: ds
22+
type: time
23+
type_params:
24+
time_granularity: day
25+
26+
primary_entity: booking

tests_metricflow/fixtures/manifest_fixtures.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
from metricflow_semantics.test_helpers.semantic_manifest_yamls.multi_hop_join_manifest import (
3333
MULTI_HOP_JOIN_MANIFEST_ANCHOR,
3434
)
35+
from metricflow_semantics.test_helpers.semantic_manifest_yamls.name_edge_case_manifest import (
36+
NAME_EDGE_CASE_MANIFEST_ANCHOR,
37+
)
3538
from metricflow_semantics.test_helpers.semantic_manifest_yamls.non_sm_manifest import NON_SM_MANIFEST_ANCHOR
3639
from metricflow_semantics.test_helpers.semantic_manifest_yamls.partitioned_multi_hop_join_manifest import (
3740
PARTITIONED_MULTI_HOP_JOIN_MANIFEST_ANCHOR,
@@ -130,6 +133,11 @@ class SemanticManifestSetup(Enum):
130133
id_number_space=IdNumberSpace.for_block(10),
131134
yaml_file_dir=SIMPLE_MULTI_HOP_JOIN_MANIFEST_ANCHOR.directory,
132135
)
136+
NAME_EDGE_CASE_MANIFEST = SemanticManifestSetupPropertySet(
137+
semantic_manifest_name="name_edge_case_manifest",
138+
id_number_space=IdNumberSpace.for_block(11),
139+
yaml_file_dir=NAME_EDGE_CASE_MANIFEST_ANCHOR.directory,
140+
)
133141

134142
@property
135143
def id_number_space(self) -> IdNumberSpace: # noqa: D102
@@ -383,3 +391,11 @@ def simple_multi_hop_join_manifest_lookup(
383391
) -> SemanticManifestLookup:
384392
"""Manifest used to test ambiguous resolution of group-by-items."""
385393
return mf_engine_test_fixture_mapping[SemanticManifestSetup.SIMPLE_MULTI_HOP_JOIN_MANIFEST].semantic_manifest_lookup
394+
395+
396+
@pytest.fixture(scope="session")
397+
def name_edge_case_manifest(
398+
mf_engine_test_fixture_mapping: Mapping[SemanticManifestSetup, MetricFlowEngineTestFixture]
399+
) -> SemanticManifestLookup:
400+
"""Manifest used to test name-related edge cases."""
401+
return mf_engine_test_fixture_mapping[SemanticManifestSetup.NAME_EDGE_CASE_MANIFEST].semantic_manifest_lookup
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
from __future__ import annotations
2+
3+
from collections.abc import Mapping
4+
5+
import pytest
6+
from _pytest.fixtures import FixtureRequest
7+
from metricflow_semantics.test_helpers.config_helpers import MetricFlowTestConfiguration
8+
9+
from metricflow.protocols.sql_client import SqlClient
10+
from tests_metricflow.fixtures.manifest_fixtures import MetricFlowEngineTestFixture, SemanticManifestSetup
11+
from tests_metricflow.query_rendering.compare_rendered_query import render_and_check
12+
13+
14+
@pytest.mark.sql_engine_snapshot
15+
@pytest.mark.duckdb_only
16+
def test_metric_name_same_as_dimension_name(
17+
request: FixtureRequest,
18+
mf_test_configuration: MetricFlowTestConfiguration,
19+
mf_engine_test_fixture_mapping: Mapping[SemanticManifestSetup, MetricFlowEngineTestFixture],
20+
sql_client: SqlClient,
21+
) -> None:
22+
"""Check a soon-to-be-deprecated use case where a manifest contains a metric with the same name as a dimension."""
23+
fixture = mf_engine_test_fixture_mapping[SemanticManifestSetup.NAME_EDGE_CASE_MANIFEST]
24+
render_and_check(
25+
request=request,
26+
mf_test_configuration=mf_test_configuration,
27+
dataflow_to_sql_converter=fixture.dataflow_to_sql_converter,
28+
sql_client=sql_client,
29+
dataflow_plan_builder=fixture.dataflow_plan_builder,
30+
query_spec=fixture.query_parser.parse_and_validate_query(
31+
metric_names=["conflicted_name"], group_by_names=["booking__conflicted_name"]
32+
).query_spec,
33+
)

tests_metricflow/snapshots/test_cli.py/str/test_saved_query_explain__result.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ test_filename: test_cli.py
66
SELECT
77
subq_1.metric_time__day AS metric_time__day
88
, customers_src_10000.country AS customer__customer_country
9-
, SUM(subq_1.transactions) AS transactions
10-
, SUM(subq_1.quick_buy_transactions) AS quick_buy_transactions
9+
, SUM(subq_1.__transactions) AS transactions
10+
, SUM(subq_1.__quick_buy_transactions) AS quick_buy_transactions
1111
FROM (
1212
SELECT
1313
DATE_TRUNC('day', ds) AS metric_time__day
1414
, id_customer AS customer
15-
, CASE WHEN CASE WHEN transaction_type_name = 'quick-buy' THEN TRUE ELSE FALSE END THEN 1 ELSE 0 END AS quick_buy_transactions
16-
, 1 AS transactions
15+
, CASE WHEN CASE WHEN transaction_type_name = 'quick-buy' THEN TRUE ELSE FALSE END THEN 1 ELSE 0 END AS __quick_buy_transactions
16+
, 1 AS __transactions
1717
FROM "mf_tutorial"."main"."transactions" transactions_src_10000
1818
) subq_1
1919
LEFT OUTER JOIN

tests_metricflow/snapshots/test_cli_quiet.py/str/test_explain__result.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ docstring:
55
---
66
SELECT
77
metric_time__day
8-
, SUM(transactions) AS transactions
8+
, SUM(__transactions) AS transactions
99
FROM (
1010
SELECT
1111
DATE_TRUNC('day', ds) AS metric_time__day
12-
, 1 AS transactions
12+
, 1 AS __transactions
1313
FROM "mf_tutorial"."main"."transactions" transactions_src_10000
1414
) subq_2
1515
GROUP BY

0 commit comments

Comments
 (0)