Fix filter parsing bug (#9508)

* Use Union syntax that works for dataclasses

* Changelog

* Use simpler syntax

* Add comments explaining this for future devs

* Write tests

.changes/unreleased/Fixes-20240201-164407.yaml

@@ -0,0 +1,6 @@
+kind: Fixes
+body: Fix bug where Semantic Layer filter strings are parsed into lists.
+time: 2024-02-01T16:44:07.697777-08:00
+custom:
+  Author: courtneyholcomb
+  Issue: "9507"

core/dbt/contracts/graph/unparsed.py

@@ -482,7 +482,8 @@ def __bool__(self):
 @dataclass
 class UnparsedMetricInputMeasure(dbtClassMixin):
     name: str
-    filter: Optional[Union[str, List[str]]] = None
+    # Note: `Union` must be the outermost part of the type annotation for serialization to work properly.
+    filter: Union[str, List[str], None] = None
     alias: Optional[str] = None
     join_to_timespine: bool = False
     fill_nulls_with: Optional[int] = None
@@ -491,7 +492,8 @@ class UnparsedMetricInputMeasure(dbtClassMixin):
 @dataclass
 class UnparsedMetricInput(dbtClassMixin):
     name: str
-    filter: Optional[Union[str, List[str]]] = None
+    # Note: `Union` must be the outermost part of the type annotation for serialization to work properly.
+    filter: Union[str, List[str], None] = None
     alias: Optional[str] = None
     offset_window: Optional[str] = None
     offset_to_grain: Optional[str] = None  # str is really a TimeGranularity Enum
@@ -528,7 +530,8 @@ class UnparsedMetric(dbtClassMixin):
     type: str
     type_params: UnparsedMetricTypeParams
     description: str = ""
-    filter: Optional[Union[str, List[str]]] = None
+    # Note: `Union` must be the outermost part of the type annotation for serialization to work properly.
+    filter: Union[str, List[str], None] = None
     # metadata: Optional[Unparsedetadata] = None # TODO
     meta: Dict[str, Any] = field(default_factory=dict)
     tags: List[str] = field(default_factory=list)
@@ -638,7 +641,8 @@ class UnparsedSemanticModel(dbtClassMixin):
 class UnparsedQueryParams(dbtClassMixin):
     metrics: List[str] = field(default_factory=list)
     group_by: List[str] = field(default_factory=list)
-    where: Optional[Union[str, List[str]]] = None
+    # Note: `Union` must be the outermost part of the type annotation for serialization to work properly.
+    where: Union[str, List[str], None] = None
 
 
 @dataclass

tests/functional/metrics/fixtures.py

@@ -748,3 +748,38 @@
           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"
+"""

tests/functional/metrics/test_metrics.py

@@ -28,6 +28,8 @@
     semantic_model_people_yml,
     semantic_model_purchasing_yml,
     purchasing_model_sql,
+    filtered_metrics_yml,
+    basic_metrics_yml,
 )
 
 
@@ -399,3 +401,53 @@ def test_conversion_metric(
             ].type_params.conversion_type_params.entity
             == "purchase"
         )
+
+
+class TestFilterParsing:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "basic_metrics.yml": basic_metrics_yml,
+            "filtered_metrics.yml": filtered_metrics_yml,
+            "metricflow_time_spine.sql": metricflow_time_spine_sql,
+            "semantic_model_people.yml": semantic_model_people_yml,
+            "people.sql": models_people_sql,
+        }
+
+    # Tests that filters are parsed to their appropriate type
+    def test_string_filter_parsing(
+        self,
+        project,
+    ):
+        runner = dbtRunner()
+        result = runner.invoke(["parse"])
+        assert result.success
+        assert isinstance(result.result, Manifest)
+
+        manifest = get_manifest(project.project_root)
+        assert manifest
+
+        # Test metrics with input measure filters.
+        filters1 = (
+            manifest.metrics["metric.test.collective_tenure_measure_filter_str"]
+            .input_measures[0]
+            .filter.where_filters
+        )
+        assert len(filters1) == 1
+        assert filters1[0].where_sql_template == "{{ Dimension('id__loves_dbt') }} is true"
+
+        # Test metrics with metric-level filters.
+        filters2 = manifest.metrics[
+            "metric.test.collective_tenure_metric_filter_str"
+        ].filter.where_filters
+        assert len(filters2) == 1
+        assert filters2[0].where_sql_template == "{{ Dimension('id__loves_dbt') }} is true"
+
+        # Test derived metrics with input metric filters.
+        filters3 = (
+            manifest.metrics["metric.test.average_tenure_filter_str"]
+            .input_metrics[0]
+            .filter.where_filters
+        )
+        assert len(filters3) == 1
+        assert filters3[0].where_sql_template == "{{ Dimension('id__loves_dbt') }} is true"

tests/functional/saved_queries/fixtures.py

@@ -25,6 +25,39 @@
             schema: my_export_schema_name
 """
 
+saved_queries_with_diff_filters_yml = """
+version: 2
+
+saved_queries:
+  - name: test_saved_query_where_list
+    description: "{{ doc('saved_query_description') }}"
+    label: Test Saved Query
+    query_params:
+        metrics:
+            - simple_metric
+        group_by:
+            - "Dimension('user__ds')"
+        where:
+            - "{{ Dimension('user__ds', 'DAY') }} <= now()"
+            - "{{ Dimension('user__ds', 'DAY') }} >= '2023-01-01'"
+    exports:
+        - name: my_export
+          config:
+            alias: my_export_alias
+            export_as: table
+            schema: my_export_schema_name
+
+  - name: test_saved_query_where_str
+    description: "{{ doc('saved_query_description') }}"
+    label: Test Saved Query2
+    query_params:
+      metrics:
+        - simple_metric
+      group_by:
+        - "Dimension('user__ds')"
+      where: "{{ Dimension('user__ds', 'DAY') }} <= now()"
+"""
+
 saved_query_with_extra_config_attributes_yml = """
 version: 2
 

tests/functional/saved_queries/test_saved_query_parsing.py

@@ -7,7 +7,11 @@
 from dbt.tests.util import write_file
 from dbt_semantic_interfaces.type_enums.export_destination_type import ExportDestinationType
 from tests.functional.assertions.test_runner import dbtTestRunner
-from tests.functional.saved_queries.fixtures import saved_queries_yml, saved_query_description
+from tests.functional.saved_queries.fixtures import (
+    saved_queries_yml,
+    saved_query_description,
+    saved_queries_with_diff_filters_yml,
+)
 from tests.functional.semantic_models.fixtures import (
     fct_revenue_sql,
     metricflow_time_spine_sql,
@@ -63,12 +67,38 @@ class TestSavedQueryPartialParsing:
     def models(self):
         return {
             "saved_queries.yml": saved_queries_yml,
+            "saved_queries_with_diff_filters.yml": saved_queries_with_diff_filters_yml,
             "schema.yml": schema_yml,
             "fct_revenue.sql": fct_revenue_sql,
             "metricflow_time_spine.sql": metricflow_time_spine_sql,
             "docs.md": saved_query_description,
         }
 
+    def test_saved_query_filter_types(self, project):
+        runner = dbtTestRunner()
+        result = runner.invoke(["parse"])
+        assert result.success
+
+        manifest = result.result
+        saved_query1 = manifest.saved_queries["saved_query.test.test_saved_query_where_list"]
+        saved_query2 = manifest.saved_queries["saved_query.test.test_saved_query_where_str"]
+
+        # List filter
+        assert len(saved_query1.query_params.where.where_filters) == 2
+        assert {
+            where_filter.where_sql_template
+            for where_filter in saved_query1.query_params.where.where_filters
+        } == {
+            "{{ Dimension('user__ds', 'DAY') }} <= now()",
+            "{{ Dimension('user__ds', 'DAY') }} >= '2023-01-01'",
+        }
+        # String filter
+        assert len(saved_query2.query_params.where.where_filters) == 1
+        assert (
+            saved_query2.query_params.where.where_filters[0].where_sql_template
+            == "{{ Dimension('user__ds', 'DAY') }} <= now()"
+        )
+
     def test_saved_query_metrics_changed(self, project):
         # First, use the default saved_queries.yml to define our saved_queries, and
         # run the dbt parse command