Skip to content

Commit

Permalink
Do not add duplicate input_measures when parsing metrics (#9677) (#9760)
Browse files Browse the repository at this point in the history
  • Loading branch information
gshank authored Mar 14, 2024
1 parent 88bb7a3 commit 5d0dd2a
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 5 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240226-173227.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Do not add duplicate input_measures
time: 2024-02-26T17:32:27.837427-05:00
custom:
Author: gshank
Issue: "9360"
6 changes: 6 additions & 0 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,12 @@ def same_contents(self, old: Optional["Metric"]) -> bool:
and True
)

def add_input_measure(self, input_measure: MetricInputMeasure) -> None:
for existing_input_measure in self.type_params.input_measures:
if input_measure == existing_input_measure:
return
self.type_params.input_measures.append(input_measure)


# ====================================
# Group node
Expand Down
9 changes: 5 additions & 4 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@ def _process_metric_node(
assert (
metric.type_params.measure is not None
), f"{metric} should have a measure defined, but it does not."
metric.type_params.input_measures.append(metric.type_params.measure)
metric.add_input_measure(metric.type_params.measure)
_process_metric_depends_on(
manifest=manifest, current_project=current_project, metric=metric
)
Expand All @@ -1581,8 +1581,8 @@ def _process_metric_node(
assert (
conversion_type_params
), f"{metric.name} is a conversion metric and must have conversion_type_params defined."
metric.type_params.input_measures.append(conversion_type_params.base_measure)
metric.type_params.input_measures.append(conversion_type_params.conversion_measure)
metric.add_input_measure(conversion_type_params.base_measure)
metric.add_input_measure(conversion_type_params.conversion_measure)
_process_metric_depends_on(
manifest=manifest, current_project=current_project, metric=metric
)
Expand Down Expand Up @@ -1618,7 +1618,8 @@ def _process_metric_node(
_process_metric_node(
manifest=manifest, current_project=current_project, metric=target_metric
)
metric.type_params.input_measures.extend(target_metric.type_params.input_measures)
for input_measure in target_metric.type_params.input_measures:
metric.add_input_measure(input_measure)
metric.depends_on.add_node(target_metric.unique_id)
else:
assert_values_exhausted(metric.type)
Expand Down
53 changes: 53 additions & 0 deletions tests/functional/metrics/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,3 +664,56 @@
conversion_measure: num_orders
entity: purchase
"""

filtered_metrics_yml = """
version: 2
metrics:
- name: collective_tenure_measure_filter_str
label: "Collective tenure1"
description: Total number of years of team experience
type: simple
type_params:
measure:
name: "years_tenure"
filter: "{{ Dimension('id__loves_dbt') }} is true"
- name: collective_tenure_metric_filter_str
label: Collective tenure3
description: Total number of years of team experience
type: simple
type_params:
measure:
name: "years_tenure"
filter: "{{ Dimension('id__loves_dbt') }} is true"
- name: average_tenure_filter_str
label: Average tenure of people who love dbt1
description: Average tenure of people who love dbt
type: derived
type_params:
expr: "average_tenure"
metrics:
- name: average_tenure
filter: "{{ Dimension('id__loves_dbt') }} is true"
"""

duplicate_measure_metric_yml = """
metrics:
# Simple metrics
- name: people_with_tenure
description: "Count of people with tenure"
type: simple
label: People with tenure
type_params:
measure: people
- name: ratio_tenure_to_people
description: People to years of tenure
label: New customers to all customers
type: ratio
type_params:
numerator: people_with_tenure
denominator: number_of_people
"""
22 changes: 21 additions & 1 deletion tests/functional/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
semantic_model_people_yml,
semantic_model_purchasing_yml,
purchasing_model_sql,
basic_metrics_yml,
duplicate_measure_metric_yml,
)


Expand Down Expand Up @@ -76,7 +78,7 @@ def test_simple_metric(
"metric.test.average_tenure_minus_people"
].type_params.input_measures
)
== 3
== 2
)


Expand Down Expand Up @@ -399,3 +401,21 @@ def test_conversion_metric(
].type_params.conversion_type_params.entity
== "purchase"
)


class TestDuplicateInputMeasures:
@pytest.fixture(scope="class")
def models(self):
return {
"basic_metrics.yml": basic_metrics_yml,
"filtered_metrics.yml": duplicate_measure_metric_yml,
"metricflow_time_spine.sql": metricflow_time_spine_sql,
"semantic_model_people.yml": semantic_model_people_yml,
"people.sql": models_people_sql,
}

def test_duplicate_input_measures(self, project):
runner = dbtRunner()
result = runner.invoke(["parse"])
assert result.success
assert isinstance(result.result, Manifest)

0 comments on commit 5d0dd2a

Please sign in to comment.