diff --git a/.changes/unreleased/Features-20240927-154038.yaml b/.changes/unreleased/Features-20240927-154038.yaml new file mode 100644 index 00000000..5456b6fa --- /dev/null +++ b/.changes/unreleased/Features-20240927-154038.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Added validation warning when using filter on conversion input measure +time: 2024-09-27T15:40:38.968626-04:00 +custom: + Author: WilliamDee + Issue: None diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index 3f7b540e..3ef10e37 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/dbt_semantic_interfaces/validations/metrics.py @@ -11,6 +11,7 @@ ConversionTypeParams, Dimension, Metric, + MetricInputMeasure, SemanticManifest, SemanticManifestT, SemanticModel, @@ -494,10 +495,12 @@ def _validate_measures( ) -> List[ValidationIssue]: issues: List[ValidationIssue] = [] - def _validate_measure(measure_reference: MeasureReference, semantic_model: SemanticModel) -> None: + def _validate_measure( + input_measure: MetricInputMeasure, semantic_model: SemanticModel, is_base_measure: bool = True + ) -> None: measure = None for model_measure in semantic_model.measures: - if model_measure.reference == measure_reference: + if model_measure.reference == input_measure.measure_reference: measure = model_measure break @@ -519,17 +522,31 @@ def _validate_measure(measure_reference: MeasureReference, semantic_model: Seman ) ) + if input_measure.filter is not None and not is_base_measure: + # Filters for conversion measure input are not fully supported. + issues.append( + ValidationWarning( + context=MetricContext( + file_context=FileContext.from_metadata(metadata=metric.metadata), + metric=MetricModelReference(metric_name=metric.name), + ), + message=f"Measure input {measure.name} has a filter. For conversion metrics," + " filtering on a conversion input measure is not fully supported yet.", + ) + ) + conversion_type_params = metric.type_params.conversion_type_params assert ( conversion_type_params is not None ), "For a conversion metric, type_params.conversion_type_params must exist." _validate_measure( - measure_reference=conversion_type_params.base_measure.measure_reference, + input_measure=conversion_type_params.base_measure, semantic_model=base_semantic_model, ) _validate_measure( - measure_reference=conversion_type_params.conversion_measure.measure_reference, + input_measure=conversion_type_params.conversion_measure, semantic_model=conversion_semantic_model, + is_base_measure=False, ) return issues diff --git a/tests/validations/test_metrics.py b/tests/validations/test_metrics.py index d3b61ba4..eba08083 100644 --- a/tests/validations/test_metrics.py +++ b/tests/validations/test_metrics.py @@ -1,4 +1,5 @@ from copy import deepcopy +from typing import List, Tuple import pytest @@ -54,10 +55,21 @@ ) from dbt_semantic_interfaces.validations.validator_helpers import ( SemanticManifestValidationException, + ValidationIssue, ) from tests.example_project_configuration import EXAMPLE_PROJECT_CONFIGURATION +def check_error_in_issues(error_substrings: List[str], issues: Tuple[ValidationIssue, ...]) -> None: + """Check error substrings in build issues.""" + missing_error_strings = set() + for expected_str in error_substrings: + if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in issues): + missing_error_strings.add(expected_str) + assert len(missing_error_strings) == 0, "Failed to match one or more expected issues: " + f"{missing_error_strings} in {set([x.as_readable_str() for x in issues])}" + + def test_metric_no_time_dim_dim_only_source() -> None: # noqa:D dim_name = "country" dim2_name = "ename" @@ -328,14 +340,7 @@ def test_derived_metric() -> None: # noqa: D "No input metrics found for derived metric", "No `expr` set for derived metric", ] - missing_error_strings = set() - for expected_str in expected_substrings: - if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in build_issues): - missing_error_strings.add(expected_str) - assert len(missing_error_strings) == 0, ( - f"Failed to match one or more expected errors: {missing_error_strings} in " - f"{set([x.as_readable_str() for x in build_issues])}" - ) + check_error_in_issues(error_substrings=expected_substrings, issues=build_issues) def test_where_filter_validations_happy( # noqa: D @@ -578,24 +583,43 @@ def test_conversion_metrics() -> None: # noqa: D ) ), ), + metric_with_guaranteed_meta( + name="filter_on_conversion_measure", + type=MetricType.CONVERSION, + type_params=PydanticMetricTypeParams( + conversion_type_params=PydanticConversionTypeParams( + base_measure=PydanticMetricInputMeasure(name=base_measure_name), + conversion_measure=PydanticMetricInputMeasure( + name=conversion_measure_name, + filter=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="""{{ dimension('some_bool') }}""") + ] + ), + ), + window=window, + entity=entity, + ) + ), + ), ], project_configuration=EXAMPLE_PROJECT_CONFIGURATION, ) ) - build_issues = result.errors - assert len(build_issues) == 5 - expected_substr1 = f"{invalid_entity} not found in base semantic model" - expected_substr2 = f"{invalid_entity} not found in conversion semantic model" - expected_substr3 = "the measure must be COUNT/SUM(1)/COUNT_DISTINCT" - expected_substr4 = "The provided constant property: bad_dim, cannot be found" - expected_substr5 = "The provided constant property: bad_dim2, cannot be found" - missing_error_strings = set() - for expected_str in [expected_substr1, expected_substr2, expected_substr3, expected_substr4, expected_substr5]: - if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in build_issues): - missing_error_strings.add(expected_str) - assert len(missing_error_strings) == 0, "Failed to match one or more expected errors: " - f"{missing_error_strings} in {set([x.as_readable_str() for x in build_issues])}" + build_issues = result.all_issues + assert len(result.errors) == 5 + assert len(result.warnings) == 1 + + expected_substrings = [ + f"{invalid_entity} not found in base semantic model", + f"{invalid_entity} not found in conversion semantic model", + "the measure must be COUNT/SUM(1)/COUNT_DISTINCT", + "The provided constant property: bad_dim, cannot be found", + "The provided constant property: bad_dim2, cannot be found", + "filtering on a conversion input measure is not fully supported yet", + ] + check_error_in_issues(error_substrings=expected_substrings, issues=build_issues) def test_cumulative_metrics() -> None: # noqa: D @@ -715,17 +739,14 @@ def test_cumulative_metrics() -> None: # noqa: D build_issues = validation_results.all_issues assert len(build_issues) == 4 - expected_substr1 = "Both window and grain_to_date set for cumulative metric. Please set one or the other." - expected_substr2 = "Got differing values for `window`" - expected_substr3 = "Got differing values for `grain_to_date`" - expected_substr4 = "Cumulative fields `type_params.window` and `type_params.grain_to_date` have been moved" - expected_substr5 = str(sorted({"what_a_metric", "dis_bad", "woooooo", "metric1", "metric2"})) - missing_error_strings = set() - for expected_str in [expected_substr1, expected_substr2, expected_substr3, expected_substr4, expected_substr5]: - if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in build_issues): - missing_error_strings.add(expected_str) - assert len(missing_error_strings) == 0, "Failed to match one or more expected issues: " - f"{missing_error_strings} in {set([x.as_readable_str() for x in build_issues])}" + expected_substrings = [ + "Both window and grain_to_date set for cumulative metric. Please set one or the other.", + "Got differing values for `window`", + "Got differing values for `grain_to_date`", + "Cumulative fields `type_params.window` and `type_params.grain_to_date` have been moved", + str(sorted({"what_a_metric", "dis_bad", "woooooo", "metric1", "metric2"})), + ] + check_error_in_issues(error_substrings=expected_substrings, issues=build_issues) def test_time_granularity() -> None: @@ -829,11 +850,8 @@ def test_time_granularity() -> None: build_issues = validation_results.all_issues assert len(build_issues) == 2 - expected_substr1 = "`time_granularity` for metric 'month_metric_with_invalid_time_granularity' must be >= MONTH." - expected_substr2 = "`time_granularity` for metric 'derived_metric_with_invalid_time_granularity' must be >= MONTH." - missing_error_strings = set() - for expected_str in [expected_substr1, expected_substr2]: - if not any(actual_str.as_readable_str().find(expected_str) != -1 for actual_str in build_issues): - missing_error_strings.add(expected_str) - assert len(missing_error_strings) == 0, "Failed to match one or more expected issues: " - f"{missing_error_strings} in {set([x.as_readable_str() for x in build_issues])}" + expected_substrings = [ + "`time_granularity` for metric 'month_metric_with_invalid_time_granularity' must be >= MONTH.", + "`time_granularity` for metric 'derived_metric_with_invalid_time_granularity' must be >= MONTH.", + ] + check_error_in_issues(error_substrings=expected_substrings, issues=build_issues)