From b7f84b8a0f1bcef61caa68c581a9f809fea78063 Mon Sep 17 00:00:00 2001 From: Patrick Yost Date: Sun, 27 Oct 2024 17:44:57 -0700 Subject: [PATCH] Centralize Where filter validation + comments This is not *exactly* what was described in Courtney's comments, but I think there was a little roughness in that plan once I began implementing (but I'm happy to change course if reviewers disagree!). This commit promotes the `WhereFiltersAreParseable` to be `WhereFiltersAreParseableRule` (a free-standing 'rule' in a separate file). It was weird to be passing things in from other classes but somehow centralizing the manifest, so instead, I just moved ALL of the relevant checks here. This moves the tests for where filters to a new file specifically for this rule (again, I'm open to the idea that this would be better just divided amongst the existing tests, but they share so much conceptually that it seems nice to group them together and to have a test that is pointed 1:1 at a rule where possible). Finally, I also moved some of the test support functions (`check_only_one_error_with_message`, for example) to `dbt_semantic_interfaces.test_utils` because they seem useful and reusable. --- .../Under the Hood-20241023-180425.yaml | 2 +- dbt_semantic_interfaces/test_utils.py | 39 +++ .../validations/metrics.py | 175 +--------- .../validations/saved_query.py | 70 ---- .../semantic_manifest_validator.py | 6 +- .../validations/where_filters.py | 268 +++++++++++++++ tests/validations/test_metrics.py | 142 -------- tests/validations/test_saved_query.py | 111 +------ .../test_where_filters_are_parseable.py | 309 ++++++++++++++++++ 9 files changed, 623 insertions(+), 499 deletions(-) create mode 100644 dbt_semantic_interfaces/validations/where_filters.py create mode 100644 tests/validations/test_where_filters_are_parseable.py diff --git a/.changes/unreleased/Under the Hood-20241023-180425.yaml b/.changes/unreleased/Under the Hood-20241023-180425.yaml index 33ebed54..25b933b8 100644 --- a/.changes/unreleased/Under the Hood-20241023-180425.yaml +++ b/.changes/unreleased/Under the Hood-20241023-180425.yaml @@ -1,5 +1,5 @@ kind: Under the Hood -body: Added validation warnings for invalid time spines in where filters of saved queries. +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 diff --git a/dbt_semantic_interfaces/test_utils.py b/dbt_semantic_interfaces/test_utils.py index ced0cd21..88cdd625 100644 --- a/dbt_semantic_interfaces/test_utils.py +++ b/dbt_semantic_interfaces/test_utils.py @@ -25,6 +25,9 @@ ) 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, +) logger = logging.getLogger(__name__) @@ -169,3 +172,39 @@ def semantic_model_with_guaranteed_meta( dimensions=dimensions, metadata=metadata, ) + + +def check_only_one_error_with_message( # noqa: D + results: SemanticManifestValidationResults, target_message: str +) -> None: + assert len(results.warnings) == 0 + assert len(results.errors) == 1 + assert len(results.future_errors) == 0 + + found_match = results.errors[0].message.find(target_message) != -1 + # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. + assert { + "expected": target_message, + "actual": results.errors[0].message, + } and found_match + + +def check_only_one_warning_with_message( # noqa: D + results: SemanticManifestValidationResults, target_message: str +) -> None: + assert len(results.errors) == 0 + assert len(results.warnings) == 1 + assert len(results.future_errors) == 0 + + found_match = results.warnings[0].message.find(target_message) != -1 + # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. + assert { + "expected": target_message, + "actual": results.warnings[0].message, + } and found_match + + +def check_no_errors_or_warnings(results: SemanticManifestValidationResults) -> None: # noqa: D + assert len(results.errors) == 0 + assert len(results.warnings) == 0 + assert len(results.future_errors) == 0 diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index f0eb0dff..da9153a8 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/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,7 +34,6 @@ ValidationError, ValidationIssue, ValidationWarning, - generate_exception_issue, validate_safely, ) @@ -244,177 +242,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.""" diff --git a/dbt_semantic_interfaces/validations/saved_query.py b/dbt_semantic_interfaces/validations/saved_query.py index e062b39f..c805aa8f 100644 --- a/dbt_semantic_interfaces/validations/saved_query.py +++ b/dbt_semantic_interfaces/validations/saved_query.py @@ -23,7 +23,6 @@ ) from dbt_semantic_interfaces.protocols import SemanticManifestT from dbt_semantic_interfaces.protocols.saved_query import SavedQuery -from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity from dbt_semantic_interfaces.validations.validator_helpers import ( FileContext, SavedQueryContext, @@ -31,7 +30,6 @@ SemanticManifestValidationRule, ValidationError, ValidationIssue, - ValidationWarning, generate_exception_issue, validate_safely, ) @@ -114,68 +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, custom_granularity_names: list[str]) -> 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__)), - }, - ) - ) - else: - issues += SavedQueryRule._check_where_timespine(saved_query, custom_granularity_names) - - return issues - - def _check_where_timespine( - saved_query: SavedQuery, custom_granularity_names: list[str] - ) -> Sequence[ValidationIssue]: - where_param = saved_query.query_params.where - if where_param is None: - return [] - - issues: List[ValidationIssue] = [] - - valid_granularity_names = [ - standard_granularity.name for standard_granularity in TimeGranularity - ] + custom_granularity_names - - for where_filter in where_param.where_filters: - for time_dim_call_parameter_set in where_filter.call_parameter_sets.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 not in valid_granularity_names: - issues.append( - ValidationWarning( - context=SavedQueryContext( - file_context=FileContext.from_metadata(metadata=saved_query.metadata), - element_type=SavedQueryElementType.WHERE, - element_value=where_filter.where_sql_template, - ), - # message=f"Filter for metric `{context.metric.metric_name}` is not valid. " - message=f"Filter for saved query `{saved_query.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 def _parse_query_item( saved_query: SavedQuery, @@ -317,11 +253,6 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati for entity in semantic_model.entities: valid_group_by_element_names.add(entity.name) - custom_granularity_names = [ - granularity.name - for time_spine in semantic_manifest.project_configuration.time_spines - for granularity in time_spine.custom_granularities - ] for saved_query in semantic_manifest.saved_queries: issues += SavedQueryRule._check_metrics( valid_metric_names=valid_metric_names, @@ -331,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, custom_granularity_names) issues += SavedQueryRule._check_order_by(saved_query) issues += SavedQueryRule._check_limit(saved_query) return issues diff --git a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py index 26f81452..cfad51ed 100644 --- a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py +++ b/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,9 @@ SemanticManifestValidationResults, SemanticManifestValidationRule, ) +from dbt_semantic_interfaces.validations.where_filters import ( + WhereFiltersAreParseableRule, +) logger = logging.getLogger(__name__) @@ -86,7 +88,7 @@ class SemanticManifestValidator(Generic[SemanticManifestT]): SemanticModelDefaultsRule[SemanticManifestT](), PrimaryEntityRule[SemanticManifestT](), PrimaryEntityDimensionPairs[SemanticManifestT](), - WhereFiltersAreParseable[SemanticManifestT](), + WhereFiltersAreParseableRule[SemanticManifestT](), SavedQueryRule[SemanticManifestT](), MetricLabelsRule[SemanticManifestT](), SemanticModelLabelsRule[SemanticManifestT](), diff --git a/dbt_semantic_interfaces/validations/where_filters.py b/dbt_semantic_interfaces/validations/where_filters.py new file mode 100644 index 00000000..cdde79e7 --- /dev/null +++ b/dbt_semantic_interfaces/validations/where_filters.py @@ -0,0 +1,268 @@ +import traceback +from typing import Generic, List, Sequence, Tuple + +from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets +from dbt_semantic_interfaces.protocols import Metric, SemanticManifestT +from dbt_semantic_interfaces.protocols.saved_query import SavedQuery +from dbt_semantic_interfaces.references import MetricModelReference +from dbt_semantic_interfaces.type_enums import TimeGranularity +from dbt_semantic_interfaces.validations.validator_helpers import ( + FileContext, + MetricContext, + SavedQueryContext, + SavedQueryElementType, + SemanticManifestValidationRule, + ValidationContext, + ValidationIssue, + ValidationWarning, + generate_exception_issue, + validate_safely, +) + + +class WhereFiltersAreParseableRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): + """Validates that all Metric WhereFilters are parseable.""" + + @staticmethod + def _validate_time_granularity_names_impl( + location_label_for_errors: str, + filter_call_parameter_sets_with_context: Sequence[Tuple[ValidationContext, 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 context, parameter_set in filter_call_parameter_sets_with_context: + 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 `{location_label_for_errors}` 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 + def _validate_time_granularity_names_for_saved_query( + saved_query: SavedQuery, custom_granularity_names: list[str] + ) -> Sequence[ValidationIssue]: + where_param = saved_query.query_params.where + if where_param is None: + return [] + + return WhereFiltersAreParseableRule._validate_time_granularity_names_impl( + location_label_for_errors="saved query `{saved_query.name}`", + filter_call_parameter_sets_with_context=[ + ( + SavedQueryContext( + file_context=FileContext.from_metadata(metadata=saved_query.metadata), + element_type=SavedQueryElementType.WHERE, + element_value=where_filter.where_sql_template, + ), + where_filter.call_parameter_sets, + ) + for where_filter in where_param.where_filters + ], + custom_granularity_names=custom_granularity_names, + ) + + @staticmethod + def _validate_time_granularity_names_for_metric( + context: MetricContext, + filter_expression_parameter_sets: Sequence[Tuple[str, FilterCallParameterSets]], + custom_granularity_names: list[str], + ) -> Sequence[ValidationIssue]: + return WhereFiltersAreParseableRule._validate_time_granularity_names_impl( + location_label_for_errors="metric `{context.metric.metric_name}`", + filter_call_parameter_sets_with_context=[ + ( + context, + param_set[1], + ) + for param_set in filter_expression_parameter_sets + ], + custom_granularity_names=custom_granularity_names, + ) + + @staticmethod + @validate_safely("validating the where field in a saved query.") + def _validate_saved_query( + saved_query: SavedQuery, custom_granularity_names: list[str] + ) -> 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__)), + }, + ) + ) + else: + issues += WhereFiltersAreParseableRule._validate_time_granularity_names_for_saved_query( + saved_query, custom_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 += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + 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 += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + 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 += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + 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 += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + 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 += WhereFiltersAreParseableRule._validate_time_granularity_names_for_metric( + context=context, + filter_expression_parameter_sets=input_metric.filter.filter_expression_parameter_sets, + custom_granularity_names=custom_granularity_names, + ) + 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 += WhereFiltersAreParseableRule._validate_metric( + metric=metric, custom_granularity_names=custom_granularity_names + ) + for saved_query in semantic_manifest.saved_queries: + issues += WhereFiltersAreParseableRule._validate_saved_query(saved_query, custom_granularity_names) + + return issues diff --git a/tests/validations/test_metrics.py b/tests/validations/test_metrics.py index efafeef0..c96d23b3 100644 --- a/tests/validations/test_metrics.py +++ b/tests/validations/test_metrics.py @@ -1,4 +1,3 @@ -from copy import deepcopy from typing import List, Tuple import pytest @@ -31,7 +30,6 @@ TimeDimensionReference, ) from dbt_semantic_interfaces.test_utils import ( - find_metric_with, metric_with_guaranteed_meta, semantic_model_with_guaranteed_meta, ) @@ -48,7 +46,6 @@ CumulativeMetricRule, DerivedMetricRule, MetricTimeGranularityRule, - WhereFiltersAreParseable, ) from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( SemanticManifestValidator, @@ -345,145 +342,6 @@ def test_derived_metric() -> None: # noqa: D check_error_in_issues(error_substrings=expected_substrings, issues=build_issues) -def test_where_filter_validations_happy( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - results = validator.validate_semantic_manifest(simple_semantic_manifest__with_primary_transforms) - assert not results.has_blocking_issues - - -def test_where_filter_validations_bad_base_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with(manifest, lambda metric: metric.filter is not None) - assert metric.filter is not None - assert len(metric.filter.where_filters) > 0 - metric.filter.where_filters[0].where_sql_template = "{{ dimension('too', 'many', 'variables', 'to', 'handle') }}" - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises(SemanticManifestValidationException, match=f"trying to parse filter of metric `{metric.name}`"): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_measure_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, lambda metric: metric.type_params is not None and metric.type_params.measure is not None - ) - assert metric.type_params.measure is not None - metric.type_params.measure.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, - match=f"trying to parse filter of measure input `{metric.type_params.measure.name}` on metric `{metric.name}`", - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_numerator_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, lambda metric: metric.type_params is not None and metric.type_params.numerator is not None - ) - assert metric.type_params.numerator is not None - metric.type_params.numerator.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, match=f"trying to parse the numerator filter on metric `{metric.name}`" - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_denominator_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, lambda metric: metric.type_params is not None and metric.type_params.denominator is not None - ) - assert metric.type_params.denominator is not None - metric.type_params.denominator.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, match=f"trying to parse the denominator filter on metric `{metric.name}`" - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_bad_input_metric_filter( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, - lambda metric: metric.type_params is not None - and metric.type_params.metrics is not None - and len(metric.type_params.metrics) > 0, - ) - assert metric.type_params.metrics is not None - input_metric = metric.type_params.metrics[0] - input_metric.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - with pytest.raises( - SemanticManifestValidationException, - match=f"trying to parse filter for input metric `{input_metric.name}` on metric `{metric.name}`", - ): - validator.checked_validations(manifest) - - -def test_where_filter_validations_invalid_granularity( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = deepcopy(simple_semantic_manifest__with_primary_transforms) - - metric, _ = find_metric_with( - manifest, - lambda metric: metric.type_params is not None - and metric.type_params.metrics is not None - and len(metric.type_params.metrics) > 0, - ) - assert metric.type_params.metrics is not None - input_metric = metric.type_params.metrics[0] - input_metric.filter = PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'month') }}"), - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'MONTH') }}"), - ] - ) - validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseable()]) - issues = validator.validate_semantic_manifest(manifest) - assert not issues.has_blocking_issues - assert len(issues.warnings) == 1 - assert "`cool` is not a valid granularity name" in issues.warnings[0].message - - def test_conversion_metrics() -> None: # noqa: D base_measure_name = "base_measure" conversion_measure_name = "conversion_measure" diff --git a/tests/validations/test_saved_query.py b/tests/validations/test_saved_query.py index 56f50dea..566407f9 100644 --- a/tests/validations/test_saved_query.py +++ b/tests/validations/test_saved_query.py @@ -12,47 +12,15 @@ from dbt_semantic_interfaces.implementations.semantic_manifest import ( PydanticSemanticManifest, ) +from dbt_semantic_interfaces.test_utils import check_only_one_error_with_message from dbt_semantic_interfaces.validations.saved_query import SavedQueryRule from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( SemanticManifestValidator, ) -from dbt_semantic_interfaces.validations.validator_helpers import ( - SemanticManifestValidationResults, -) logger = logging.getLogger(__name__) -def check_only_one_error_with_message( # noqa: D - results: SemanticManifestValidationResults, target_message: str -) -> None: - assert len(results.warnings) == 0 - assert len(results.errors) == 1 - assert len(results.future_errors) == 0 - - found_match = results.errors[0].message.find(target_message) != -1 - # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. - assert { - "expected": target_message, - "actual": results.errors[0].message, - } and found_match - - -def check_only_one_warning_with_message( # noqa: D - results: SemanticManifestValidationResults, target_message: str -) -> None: - assert len(results.warnings) == 1 - assert len(results.errors) == 0 - assert len(results.future_errors) == 0 - - found_match = results.warnings[0].message.find(target_message) != -1 - # Adding this dict to the assert so that when it does not match, pytest prints the expected and actual values. - assert { - "expected": target_message, - "actual": results.warnings[0].message, - } and found_match - - def test_invalid_metric_in_saved_query( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: @@ -77,59 +45,6 @@ def test_invalid_metric_in_saved_query( # noqa: D ) -def test_invalid_where_in_saved_query( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) - manifest.saved_queries = [ - PydanticSavedQuery( - name="Example Saved Query", - description="Example description.", - query_params=PydanticSavedQueryQueryParams( - metrics=["bookings"], - group_by=["Dimension('booking__is_instant')"], - where=PydanticWhereFilterIntersection( - where_filters=[PydanticWhereFilter(where_sql_template="{{ invalid_jinja }}")], - ), - ), - ), - ] - - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) - check_only_one_error_with_message( - manifest_validator.validate_semantic_manifest(manifest), - "trying to parse a filter in saved query", - ) - - -def test_where_filter_validations_invalid_granularity( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) - - manifest.saved_queries = [ - PydanticSavedQuery( - name="Example Saved Query", - description="Example description.", - query_params=PydanticSavedQueryQueryParams( - metrics=["bookings"], - group_by=["Dimension('booking__is_instant')"], - where=PydanticWhereFilterIntersection( - where_filters=[ - PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), - ] - ), - ), - ), - ] - - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) - check_only_one_warning_with_message( - manifest_validator.validate_semantic_manifest(manifest), - "is not a valid granularity name", - ) - - def test_invalid_group_by_element_in_saved_query( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: @@ -180,30 +95,6 @@ def test_invalid_group_by_format_in_saved_query( # noqa: D ) -def test_metric_filter_error( # noqa: D - simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, -) -> None: - manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) - manifest.saved_queries = [ - PydanticSavedQuery( - name="Example Saved Query", - description="Example description.", - query_params=PydanticSavedQueryQueryParams( - metrics=["listings"], - where=PydanticWhereFilterIntersection( - where_filters=[PydanticWhereFilter(where_sql_template="{{ Metric('bookings') }} > 2")], - ), - ), - ), - ] - - manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([SavedQueryRule()]) - check_only_one_error_with_message( - manifest_validator.validate_semantic_manifest(manifest), - "An error occurred while trying to parse a filter in saved query", - ) - - def test_metric_filter_success( # noqa: D simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, ) -> None: diff --git a/tests/validations/test_where_filters_are_parseable.py b/tests/validations/test_where_filters_are_parseable.py new file mode 100644 index 00000000..15b75986 --- /dev/null +++ b/tests/validations/test_where_filters_are_parseable.py @@ -0,0 +1,309 @@ +import copy +import logging + +import pytest + +from dbt_semantic_interfaces.implementations.filters.where_filter import ( + PydanticWhereFilter, + PydanticWhereFilterIntersection, +) +from dbt_semantic_interfaces.implementations.saved_query import ( + PydanticSavedQuery, + PydanticSavedQueryQueryParams, +) +from dbt_semantic_interfaces.implementations.semantic_manifest import ( + PydanticSemanticManifest, +) +from dbt_semantic_interfaces.test_utils import ( + check_no_errors_or_warnings, + check_only_one_error_with_message, + check_only_one_warning_with_message, + find_metric_with, +) +from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( + SemanticManifestValidator, +) +from dbt_semantic_interfaces.validations.validator_helpers import ( + SemanticManifestValidationException, +) +from dbt_semantic_interfaces.validations.where_filters import ( + WhereFiltersAreParseableRule, +) + +logger = logging.getLogger(__name__) + + +# ------------------------------------------------------------------------------ +# Metric validations +# ------------------------------------------------------------------------------ + + +def test_metric_where_filter_validations_happy( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + results = validator.validate_semantic_manifest(simple_semantic_manifest__with_primary_transforms) + assert not results.has_blocking_issues + + +def test_where_filter_validations_bad_base_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with(manifest, lambda metric: metric.filter is not None) + assert metric.filter is not None + assert len(metric.filter.where_filters) > 0 + metric.filter.where_filters[0].where_sql_template = "{{ dimension('too', 'many', 'variables', 'to', 'handle') }}" + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises(SemanticManifestValidationException, match=f"trying to parse filter of metric `{metric.name}`"): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_measure_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, lambda metric: metric.type_params is not None and metric.type_params.measure is not None + ) + assert metric.type_params.measure is not None + metric.type_params.measure.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, + match=f"trying to parse filter of measure input `{metric.type_params.measure.name}` on metric `{metric.name}`", + ): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_numerator_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, lambda metric: metric.type_params is not None and metric.type_params.numerator is not None + ) + assert metric.type_params.numerator is not None + metric.type_params.numerator.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, match=f"trying to parse the numerator filter on metric `{metric.name}`" + ): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_denominator_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, lambda metric: metric.type_params is not None and metric.type_params.denominator is not None + ) + assert metric.type_params.denominator is not None + metric.type_params.denominator.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, match=f"trying to parse the denominator filter on metric `{metric.name}`" + ): + validator.checked_validations(manifest) + + +def test_where_filter_validations_bad_input_metric_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, + lambda metric: metric.type_params is not None + and metric.type_params.metrics is not None + and len(metric.type_params.metrics) > 0, + ) + assert metric.type_params.metrics is not None + input_metric = metric.type_params.metrics[0] + input_metric.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ dimension('too', 'many', 'variables', 'to', 'handle') }}") + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + with pytest.raises( + SemanticManifestValidationException, + match=f"trying to parse filter for input metric `{input_metric.name}` on metric `{metric.name}`", + ): + validator.checked_validations(manifest) + + +def test_metric_where_filter_validations_invalid_granularity( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + metric, _ = find_metric_with( + manifest, + lambda metric: metric.type_params is not None + and metric.type_params.metrics is not None + and len(metric.type_params.metrics) > 0, + ) + assert metric.type_params.metrics is not None + input_metric = metric.type_params.metrics[0] + input_metric.filter = PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'month') }}"), + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'MONTH') }}"), + ] + ) + validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + issues = validator.validate_semantic_manifest(manifest) + assert not issues.has_blocking_issues + assert len(issues.warnings) == 1 + assert "`cool` is not a valid granularity name" in issues.warnings[0].message + + +# ------------------------------------------------------------------------------ +# Saved Query validations +# ------------------------------------------------------------------------------ + + +def test_saved_query_with_happy_filter( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'hour') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_no_errors_or_warnings(manifest_validator.validate_semantic_manifest(manifest)) + + +def test_saved_query_validates_granularity_name_despite_case( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'DAY') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_no_errors_or_warnings(manifest_validator.validate_semantic_manifest(manifest)) + + +def test_invalid_where_in_saved_query( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[PydanticWhereFilter(where_sql_template="{{ invalid_jinja }}")], + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_only_one_error_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "trying to parse a filter in saved query", + ) + + +def test_saved_query_where_filter_validations_invalid_granularity( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["bookings"], + group_by=["Dimension('booking__is_instant')"], + where=PydanticWhereFilterIntersection( + where_filters=[ + PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time', 'cool') }}"), + ] + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_only_one_warning_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "is not a valid granularity name", + ) + + +def test_metric_filter_error( # noqa: D + simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest, +) -> None: + manifest = copy.deepcopy(simple_semantic_manifest__with_primary_transforms) + manifest.saved_queries = [ + PydanticSavedQuery( + name="Example Saved Query", + description="Example description.", + query_params=PydanticSavedQueryQueryParams( + metrics=["listings"], + where=PydanticWhereFilterIntersection( + where_filters=[PydanticWhereFilter(where_sql_template="{{ Metric('bookings') }} > 2")], + ), + ), + ), + ] + + manifest_validator = SemanticManifestValidator[PydanticSemanticManifest]([WhereFiltersAreParseableRule()]) + check_only_one_error_with_message( + manifest_validator.validate_semantic_manifest(manifest), + "An error occurred while trying to parse a filter in saved query", + )