Merge pull request #359 from dbt-labs/validate_saved_query_timespines

Validate granularity names in saved query where filters

.changes/unreleased/Under the Hood-20241023-180425.yaml

@@ -0,0 +1,6 @@
+kind: Under the Hood
+body: Added validation warnings for invalid granularity names in where filters of saved queries.
+time: 2024-10-23T18:04:25.235887-07:00
+custom:
+    Author: theyostalservice
+    Issue: "360"

dbt_semantic_interfaces/test_utils.py

@@ -25,6 +25,10 @@
 )
 from dbt_semantic_interfaces.parsing.objects import YamlConfigFile
 from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity
+from dbt_semantic_interfaces.validations.validator_helpers import (
+    SemanticManifestValidationResults,
+    ValidationIssue,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -169,3 +173,64 @@ def semantic_model_with_guaranteed_meta(
         dimensions=dimensions,
         metadata=metadata,
     )
+
+
+def _assert_expected_validation_message(  # noqa: D
+    issues: Sequence[ValidationIssue],
+    message_fragment: str,
+) -> None:
+    found_match = any([issue.message.find(message_fragment) != -1 for issue in issues])
+    # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values.
+    assert {
+        "expected": message_fragment,
+        "actual_messages": [issue.message for issue in issues],
+    } and found_match
+
+
+def check_expected_issues(  # noqa: D
+    results: SemanticManifestValidationResults,
+    num_expected_errors: int = 0,
+    num_expected_warnings: int = 0,
+    expected_error_msgs: Sequence[str] = [],
+    expected_warning_msgs: Sequence[str] = [],
+) -> None:
+    """Validates the number, type, and content of ValidationIssues.
+
+    Currently assumes zero future_errors as there are no future_errors
+    implemented, but this function can be expanded to cover those if needed.
+    """
+    assert len(results.warnings) == num_expected_warnings
+    assert len(results.errors) == num_expected_errors
+    assert len(results.future_errors) == 0, "validation function expects zero future_errors to be implemented."
+
+    for expected_error_msg in expected_error_msgs:
+        _assert_expected_validation_message(issues=results.errors, message_fragment=expected_error_msg)
+    for expected_warning_msg in expected_warning_msgs:
+        _assert_expected_validation_message(issues=results.warnings, message_fragment=expected_warning_msg)
+
+
+def check_only_one_error_with_message(  # noqa: D
+    results: SemanticManifestValidationResults, target_message: str
+) -> None:
+    check_expected_issues(
+        results=results,
+        num_expected_errors=1,
+        expected_error_msgs=[target_message],
+    )
+
+
+def check_only_one_warning_with_message(  # noqa: D
+    results: SemanticManifestValidationResults, target_message: str
+) -> None:
+    check_expected_issues(
+        results=results,
+        num_expected_warnings=1,
+        expected_warning_msgs=[target_message],
+    )
+
+
+def check_no_errors_or_warnings(results: SemanticManifestValidationResults) -> None:  # noqa: D
+    # no num arguments required since all defaults are zero
+    check_expected_issues(
+        results=results,
+    )

dbt_semantic_interfaces/validations/metrics.py

@@ -1,7 +1,6 @@
 import traceback
-from typing import Dict, Generic, List, Optional, Sequence, Tuple
+from typing import Dict, Generic, List, Optional, Sequence
 
-from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets
 from dbt_semantic_interfaces.errors import ParsingException
 from dbt_semantic_interfaces.implementations.metric import (
     PydanticMetric,
@@ -35,10 +34,14 @@
     ValidationError,
     ValidationIssue,
     ValidationWarning,
-    generate_exception_issue,
     validate_safely,
 )
 
+# Avoids breaking change from moving this class out of this file.
+from dbt_semantic_interfaces.validations.where_filters import (
+    WhereFiltersAreParseable,  # noQa
+)
+
 
 class CumulativeMetricRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]):
     """Checks that cumulative metrics are configured properly."""
@@ -244,177 +247,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
         return issues
 
 
-class WhereFiltersAreParseable(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]):
-    """Validates that all Metric WhereFilters are parseable."""
-
-    @staticmethod
-    def _validate_time_granularity_names(
-        context: MetricContext,
-        filter_expression_parameter_sets: Sequence[Tuple[str, FilterCallParameterSets]],
-        custom_granularity_names: List[str],
-    ) -> Sequence[ValidationIssue]:
-        issues: List[ValidationIssue] = []
-
-        valid_granularity_names = [
-            standard_granularity.value for standard_granularity in TimeGranularity
-        ] + custom_granularity_names
-        for _, parameter_set in filter_expression_parameter_sets:
-            for time_dim_call_parameter_set in parameter_set.time_dimension_call_parameter_sets:
-                if not time_dim_call_parameter_set.time_granularity_name:
-                    continue
-                if time_dim_call_parameter_set.time_granularity_name.lower() not in valid_granularity_names:
-                    issues.append(
-                        ValidationWarning(
-                            context=context,
-                            message=f"Filter for metric `{context.metric.metric_name}` is not valid. "
-                            f"`{time_dim_call_parameter_set.time_granularity_name}` is not a valid granularity name. "
-                            f"Valid granularity options: {valid_granularity_names}",
-                        )
-                    )
-        return issues
-
-    @staticmethod
-    @validate_safely(
-        whats_being_done="running model validation ensuring a metric's filter properties are configured properly"
-    )
-    def _validate_metric(metric: Metric, custom_granularity_names: List[str]) -> Sequence[ValidationIssue]:  # noqa: D
-        issues: List[ValidationIssue] = []
-        context = MetricContext(
-            file_context=FileContext.from_metadata(metadata=metric.metadata),
-            metric=MetricModelReference(metric_name=metric.name),
-        )
-
-        if metric.filter is not None:
-            try:
-                metric.filter.filter_expression_parameter_sets
-            except Exception as e:
-                issues.append(
-                    generate_exception_issue(
-                        what_was_being_done=f"trying to parse filter of metric `{metric.name}`",
-                        e=e,
-                        context=context,
-                        extras={
-                            "traceback": "".join(traceback.format_tb(e.__traceback__)),
-                        },
-                    )
-                )
-            else:
-                issues += WhereFiltersAreParseable._validate_time_granularity_names(
-                    context=context,
-                    filter_expression_parameter_sets=metric.filter.filter_expression_parameter_sets,
-                    custom_granularity_names=custom_granularity_names,
-                )
-
-        if metric.type_params:
-            measure = metric.type_params.measure
-            if measure is not None and measure.filter is not None:
-                try:
-                    measure.filter.filter_expression_parameter_sets
-                except Exception as e:
-                    issues.append(
-                        generate_exception_issue(
-                            what_was_being_done=f"trying to parse filter of measure input `{measure.name}` "
-                            f"on metric `{metric.name}`",
-                            e=e,
-                            context=context,
-                            extras={
-                                "traceback": "".join(traceback.format_tb(e.__traceback__)),
-                            },
-                        )
-                    )
-                else:
-                    issues += WhereFiltersAreParseable._validate_time_granularity_names(
-                        context=context,
-                        filter_expression_parameter_sets=measure.filter.filter_expression_parameter_sets,
-                        custom_granularity_names=custom_granularity_names,
-                    )
-
-            numerator = metric.type_params.numerator
-            if numerator is not None and numerator.filter is not None:
-                try:
-                    numerator.filter.filter_expression_parameter_sets
-                except Exception as e:
-                    issues.append(
-                        generate_exception_issue(
-                            what_was_being_done=f"trying to parse the numerator filter on metric `{metric.name}`",
-                            e=e,
-                            context=context,
-                            extras={
-                                "traceback": "".join(traceback.format_tb(e.__traceback__)),
-                            },
-                        )
-                    )
-                else:
-                    issues += WhereFiltersAreParseable._validate_time_granularity_names(
-                        context=context,
-                        filter_expression_parameter_sets=numerator.filter.filter_expression_parameter_sets,
-                        custom_granularity_names=custom_granularity_names,
-                    )
-
-            denominator = metric.type_params.denominator
-            if denominator is not None and denominator.filter is not None:
-                try:
-                    denominator.filter.filter_expression_parameter_sets
-                except Exception as e:
-                    issues.append(
-                        generate_exception_issue(
-                            what_was_being_done=f"trying to parse the denominator filter on metric `{metric.name}`",
-                            e=e,
-                            context=context,
-                            extras={
-                                "traceback": "".join(traceback.format_tb(e.__traceback__)),
-                            },
-                        )
-                    )
-                else:
-                    issues += WhereFiltersAreParseable._validate_time_granularity_names(
-                        context=context,
-                        filter_expression_parameter_sets=denominator.filter.filter_expression_parameter_sets,
-                        custom_granularity_names=custom_granularity_names,
-                    )
-
-            for input_metric in metric.type_params.metrics or []:
-                if input_metric.filter is not None:
-                    try:
-                        input_metric.filter.filter_expression_parameter_sets
-                    except Exception as e:
-                        issues.append(
-                            generate_exception_issue(
-                                what_was_being_done=f"trying to parse filter for input metric `{input_metric.name}` "
-                                f"on metric `{metric.name}`",
-                                e=e,
-                                context=context,
-                                extras={
-                                    "traceback": "".join(traceback.format_tb(e.__traceback__)),
-                                },
-                            )
-                        )
-                    else:
-                        issues += WhereFiltersAreParseable._validate_time_granularity_names(
-                            context=context,
-                            filter_expression_parameter_sets=input_metric.filter.filter_expression_parameter_sets,
-                            custom_granularity_names=custom_granularity_names,
-                        )
-
-            # TODO: Are saved query filters being validated? Task: SL-2932
-        return issues
-
-    @staticmethod
-    @validate_safely(whats_being_done="running manifest validation ensuring all metric where filters are parseable")
-    def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]:  # noqa: D
-        issues: List[ValidationIssue] = []
-        custom_granularity_names = [
-            granularity.name
-            for time_spine in semantic_manifest.project_configuration.time_spines
-            for granularity in time_spine.custom_granularities
-        ]
-        for metric in semantic_manifest.metrics or []:
-            issues += WhereFiltersAreParseable._validate_metric(
-                metric=metric, custom_granularity_names=custom_granularity_names
-            )
-        return issues
-
-
 class ConversionMetricRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]):
     """Checks that conversion metrics are configured properly."""
 

dbt_semantic_interfaces/validations/saved_query.py

@@ -112,33 +112,6 @@ def _check_metrics(valid_metric_names: Set[str], saved_query: SavedQuery) -> Seq
                 )
         return issues
 
-    @staticmethod
-    @validate_safely("Validate the where field in a saved query.")
-    def _check_where(saved_query: SavedQuery) -> Sequence[ValidationIssue]:
-        issues: List[ValidationIssue] = []
-        if saved_query.query_params.where is None:
-            return issues
-        for where_filter in saved_query.query_params.where.where_filters:
-            try:
-                where_filter.call_parameter_sets
-            except Exception as e:
-                issues.append(
-                    generate_exception_issue(
-                        what_was_being_done=f"trying to parse a filter in saved query `{saved_query.name}`",
-                        e=e,
-                        context=SavedQueryContext(
-                            file_context=FileContext.from_metadata(metadata=saved_query.metadata),
-                            element_type=SavedQueryElementType.WHERE,
-                            element_value=where_filter.where_sql_template,
-                        ),
-                        extras={
-                            "traceback": "".join(traceback.format_tb(e.__traceback__)),
-                        },
-                    )
-                )
-
-        return issues
-
     @staticmethod
     def _parse_query_item(
         saved_query: SavedQuery,
@@ -289,7 +262,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
                 valid_group_by_element_names=valid_group_by_element_names,
                 saved_query=saved_query,
             )
-            issues += SavedQueryRule._check_where(saved_query)
             issues += SavedQueryRule._check_order_by(saved_query)
             issues += SavedQueryRule._check_limit(saved_query)
         return issues

dbt_semantic_interfaces/validations/semantic_manifest_validator.py

@@ -27,7 +27,6 @@
     ConversionMetricRule,
     CumulativeMetricRule,
     DerivedMetricRule,
-    WhereFiltersAreParseable,
 )
 from dbt_semantic_interfaces.validations.non_empty import NonEmptyRule
 from dbt_semantic_interfaces.validations.primary_entity import PrimaryEntityRule
@@ -47,6 +46,7 @@
     SemanticManifestValidationResults,
     SemanticManifestValidationRule,
 )
+from dbt_semantic_interfaces.validations.where_filters import WhereFiltersAreParseable
 
 logger = logging.getLogger(__name__)