Bug fix: non-additive dimension with non-default grain (#1384)

Fixes a bug where non-additive dimensions with non-default grain error
because the default grain is assumed incorrectly.
Reviewing by commit is likely easiest since there are so many snapshot
changes due to changes to a test semantic model.

.changes/unreleased/Fixes-20240827-130128.yaml

@@ -0,0 +1,6 @@
+kind: Fixes
+body: 'Bug fix: ensure that granularity requested for non-additive dimension is respected.'
+time: 2024-08-27T13:01:28.743199-07:00
+custom:
+  Author: courtneyholcomb
+  Issue: "1383"

metricflow-semantics/metricflow_semantics/specs/non_additive_dimension_spec.py

@@ -5,7 +5,7 @@
 from typing import Any, Sequence, Tuple
 
 from dbt_semantic_interfaces.dataclass_serialization import SerializableDataclass
-from dbt_semantic_interfaces.type_enums import AggregationType
+from dbt_semantic_interfaces.type_enums import AggregationType, TimeGranularity
 
 from metricflow_semantics.specs.entity_spec import LinklessEntitySpec
 from metricflow_semantics.specs.instance_spec import LinkableInstanceSpec
@@ -40,9 +40,10 @@ def bucket_hash(self) -> str:
         values.extend(sorted(self.window_groupings))
         return hash_items(values)
 
-    @property
-    def linkable_specs(self) -> Sequence[LinkableInstanceSpec]:  # noqa: D102
-        return (TimeDimensionSpec.from_name(self.name),) + tuple(
+    def linkable_specs(  # noqa: D102
+        self, non_additive_dimension_grain: TimeGranularity
+    ) -> Sequence[LinkableInstanceSpec]:
+        return (TimeDimensionSpec.from_name(self.name).with_grain(non_additive_dimension_grain),) + tuple(
             LinklessEntitySpec.from_element_name(entity_name) for entity_name in self.window_groupings
         )
 

metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/accounts_source.yaml

@@ -30,11 +30,25 @@ semantic_model:
         window_groupings:
           - user
 
+    - name: total_account_balance_first_day_of_month
+      agg: sum
+      expr: account_balance
+      agg_time_dimension: ds_month
+      non_additive_dimension:
+        name: ds_month
+        window_choice: min
+      create_metric: true
+
   dimensions:
     - name: ds
       type: time
       type_params:
         time_granularity: day
+    - name: ds_month
+      type: time
+      expr: ds_month
+      type_params:
+        time_granularity: month
     - name: account_type
       type: categorical
 

metricflow-semantics/tests_metricflow_semantics/snapshots/test_linkable_spec_resolver.py/list/test_linkable_element_set_as_spec_set__set0.txt

@@ -158,6 +158,7 @@
   'user__account__user__current_account_balance_by_user',
   'user__account__user__regional_starting_balance_ratios',
   'user__account__user__total_account_balance_first_day',
+  'user__account__user__total_account_balance_first_day_of_month',
   'user__active_listings',
   'user__archived_at__day',
   'user__archived_at__extract_day',
@@ -461,6 +462,7 @@
   'user__smallest_listing',
   'user__subdaily_join_to_time_spine_metric',
   'user__total_account_balance_first_day',
+  'user__total_account_balance_first_day_of_month',
   'user__verification__user__identity_verifications',
   'user__view__user__views',
   'user__views',

metricflow-semantics/tests_metricflow_semantics/snapshots/test_semantic_model_container.py/dict/test_get_names__result0.txt

@@ -9,6 +9,7 @@
     'created_at',
     'ds',
     'ds_latest',
+    'ds_month',
     'ds_partitioned',
     'home_state',
     'home_state_latest',
@@ -46,6 +47,7 @@
     'referred_bookings',
     'smallest_listing',
     'total_account_balance_first_day',
+    'total_account_balance_first_day_of_month',
     'txn_revenue',
     'views',
     'visitors',

metricflow-semantics/tests_metricflow_semantics/snapshots/test_semantic_model_container.py/str/test_linkable_elements_for_measure__result0.txt

@@ -119,6 +119,7 @@ Model Join-Path                             Entity Links
 ('listings_latest',)                        ("('user',)", "('account', 'user')")           current_account_balance_by_user                                                       ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('account', 'user')")           regional_starting_balance_ratios                                                      ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('account', 'user')")           total_account_balance_first_day                                                       ['JOINED', 'METRIC']
+('listings_latest',)                        ("('user',)", "('account', 'user')")           total_account_balance_first_day_of_month                                              ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('listing', 'user')")           active_listings                                                                       ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('listing', 'user')")           approximate_continuous_booking_value_p99                                              ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('listing', 'user')")           approximate_discrete_booking_value_p99                                                ['JOINED', 'METRIC']
@@ -193,6 +194,7 @@ Model Join-Path                             Entity Links
 ('listings_latest',)                        ("('user',)", "('user',)")                     smallest_listing                                                                      ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('user',)")                     subdaily_join_to_time_spine_metric                                                    ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('user',)")                     total_account_balance_first_day                                                       ['JOINED', 'METRIC']
+('listings_latest',)                        ("('user',)", "('user',)")                     total_account_balance_first_day_of_month                                              ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('user',)")                     views                                                                                 ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('user',)")                     visit_buy_conversion_rate                                                             ['JOINED', 'METRIC']
 ('listings_latest',)                        ("('user',)", "('user',)")                     visit_buy_conversion_rate_7days                                                       ['JOINED', 'METRIC']

metricflow-semantics/tests_metricflow_semantics/snapshots/test_semantic_model_container.py/tuple/test_linkable_elements_for_no_metrics_query__result0.txt

@@ -7,6 +7,10 @@
   'account__ds__extract_month',
   'account__ds__extract_quarter',
   'account__ds__extract_year',
+  'account__ds_month__extract_month',
+  'account__ds_month__extract_quarter',
+  'account__ds_month__extract_year',
+  'account__ds_month__month',
   'account__user',
   'booking__ds__day',
   'booking__ds__extract_day',
@@ -104,6 +108,7 @@
   'company__user__company__smallest_listing',
   'company__user__company__subdaily_join_to_time_spine_metric',
   'company__user__company__total_account_balance_first_day',
+  'company__user__company__total_account_balance_first_day_of_month',
   'company__user__company__views',
   'company__user__company__visit_buy_conversion_rate',
   'company__user__company__visit_buy_conversion_rate_7days',
@@ -448,6 +453,7 @@
   'user__account__user__current_account_balance_by_user',
   'user__account__user__regional_starting_balance_ratios',
   'user__account__user__total_account_balance_first_day',
+  'user__account__user__total_account_balance_first_day_of_month',
   'user__active_listings',
   'user__archived_at__extract_day',
   'user__archived_at__extract_dow',
@@ -577,6 +583,7 @@
   'user__smallest_listing',
   'user__subdaily_join_to_time_spine_metric',
   'user__total_account_balance_first_day',
+  'user__total_account_balance_first_day_of_month',
   'user__verification__user__identity_verifications',
   'user__view__user__views',
   'user__views',

metricflow/dataflow/builder/dataflow_plan_builder.py

@@ -1348,8 +1348,13 @@ def __get_required_and_extraneous_linkable_specs(
         for filter_spec in filter_specs:
             linkable_spec_sets_to_merge.append(LinkableSpecSet.create_from_specs(filter_spec.linkable_specs))
         if non_additive_dimension_spec:
+            non_additive_dimension_grain = self._semantic_model_lookup.get_defined_time_granularity(
+                TimeDimensionReference(non_additive_dimension_spec.name)
+            )
             linkable_spec_sets_to_merge.append(
-                LinkableSpecSet.create_from_specs(non_additive_dimension_spec.linkable_specs)
+                LinkableSpecSet.create_from_specs(
+                    non_additive_dimension_spec.linkable_specs(non_additive_dimension_grain)
+                )
             )
 
         extraneous_linkable_specs = LinkableSpecSet.merge_iterable(linkable_spec_sets_to_merge).dedupe()
@@ -1543,14 +1548,19 @@ def _build_aggregated_measure_from_measure_source_node(
         if non_additive_dimension_spec is not None:
             # Apply semi additive join on the node
             agg_time_dimension = measure_properties.agg_time_dimension
+            non_additive_dimension_grain = self._semantic_model_lookup.get_defined_time_granularity(
+                TimeDimensionReference(non_additive_dimension_spec.name)
+            )
             queried_time_dimension_spec: Optional[
                 TimeDimensionSpec
             ] = self._find_non_additive_dimension_in_linkable_specs(
                 agg_time_dimension=agg_time_dimension,
                 linkable_specs=queried_linkable_specs.as_tuple,
                 non_additive_dimension_spec=non_additive_dimension_spec,
             )
-            time_dimension_spec = TimeDimensionSpec.from_name(non_additive_dimension_spec.name)
+            time_dimension_spec = TimeDimensionSpec.from_name(non_additive_dimension_spec.name).with_grain(
+                time_granularity=non_additive_dimension_grain
+            )
             window_groupings = tuple(
                 LinklessEntitySpec.from_element_name(name) for name in non_additive_dimension_spec.window_groupings
             )

tests_metricflow/fixtures/source_table_snapshots/simple_model/fct_accounts.yaml

@@ -3,25 +3,27 @@ table_snapshot:
   column_definitions:
     - name: ds
       type: TIME
+    - name: ds_month
+      type: TIME
     - name: user_id
       type: STRING
     - name: account_balance
       type: INT
     - name: account_type
       type: STRING
   rows:
-    - ["2020-01-01", "u0004114", "2135", "checking"]
-    - ["2020-01-01", "u0004114", "1234", "savings"]
-    - ["2020-01-01", "u0003141", "24634", "checking"]
-    - ["2020-01-01", "u0003154", "23452", "checking"]
-    - ["2020-01-02", "u1612112", "5123", "checking"]
-    - ["2020-01-02", "u0005432", "8456", "savings"]
-    - ["2020-01-02", "u0003452", "6939", "checking"]
-    - ["2020-01-03", "u0005432", "5234", "checking"]
-    - ["2020-01-03", "u0005432", "234", "savings"]
-    - ["2020-01-04", "u0005414", "5582", "savings"]
-    - ["2020-01-06", "u0004114", "1213", "savings"]
-    - ["2020-01-07", "u0004114", "523", "checking"]
-    - ["2020-01-10", "u0004114", "7434", "checkings"]
-    - ["2020-01-12", "u0003141", "12939", "checking"]
-    - ["2020-01-12", "u0003452", "35", "checking"]
+    - ["2020-01-01", "2020-01-01", "u0004114", "2135", "checking"]
+    - ["2020-01-01", "2020-01-01", "u0004114", "1234", "savings"]
+    - ["2020-01-01", "2020-01-01", "u0003141", "24634", "checking"]
+    - ["2020-01-01", "2020-01-01", "u0003154", "23452", "checking"]
+    - ["2020-01-02", "2020-01-01", "u1612112", "5123", "checking"]
+    - ["2020-01-02", "2020-01-01", "u0005432", "8456", "savings"]
+    - ["2020-01-02", "2020-01-01", "u0003452", "6939", "checking"]
+    - ["2020-01-03", "2020-01-01", "u0005432", "5234", "checking"]
+    - ["2020-01-03", "2020-01-01", "u0005432", "234", "savings"]
+    - ["2020-01-04", "2020-01-01", "u0005414", "5582", "savings"]
+    - ["2020-01-06", "2020-01-01", "u0004114", "1213", "savings"]
+    - ["2020-01-07", "2020-01-01", "u0004114", "523", "checking"]
+    - ["2020-01-10", "2020-01-01", "u0004114", "7434", "checkings"]
+    - ["2020-01-12", "2020-01-01", "u0003141", "12939", "checking"]
+    - ["2020-01-12", "2020-01-01", "u0003452", "35", "checking"]

tests_metricflow/integration/test_cases/itest_semi_additive_measure.yaml

@@ -259,3 +259,24 @@ integration_test:
       b.ds = d.ds__complete
     GROUP BY
       b.account_type
+---
+integration_test:
+  name: non_default_grain
+  description: Tests selecting a semi-additive measure with an agg_time_dimension that has non-default grain
+  model: SIMPLE_MODEL
+  metrics: ["total_account_balance_first_day_of_month"]
+  check_query: |
+    SELECT
+      SUM(subq_2.total_account_balance_first_day_of_month) AS total_account_balance_first_day_of_month
+    FROM (
+      SELECT
+        {{ render_date_trunc("ds", TimeGranularity.MONTH) }} AS ds_month__month
+        , account_balance AS total_account_balance_first_day_of_month
+      FROM {{ source_schema }}.fct_accounts
+    ) subq_2
+    INNER JOIN (
+      SELECT
+        MIN({{ render_date_trunc("ds", TimeGranularity.MONTH) }}) AS ds_month__month__complete
+      FROM {{ source_schema }}.fct_accounts
+    ) subq_4
+    ON subq_2.ds_month__month = subq_4.ds_month__month__complete

tests_metricflow/query_rendering/test_query_rendering.py

@@ -565,3 +565,26 @@ def test_min_max_metric_time_week(
         dataflow_plan_builder=dataflow_plan_builder,
         query_spec=query_spec,
     )
+
+
+@pytest.mark.sql_engine_snapshot
+def test_non_additive_dimension_with_non_default_grain(
+    request: FixtureRequest,
+    mf_test_configuration: MetricFlowTestConfiguration,
+    dataflow_plan_builder: DataflowPlanBuilder,
+    sql_client: SqlClient,
+    dataflow_to_sql_converter: DataflowToSqlQueryPlanConverter,
+) -> None:
+    """Tests querying a metric with a non-additive agg_time_dimension that has non-default granularity."""
+    query_spec = MetricFlowQuerySpec(
+        metric_specs=(MetricSpec(element_name="total_account_balance_first_day_of_month"),)
+    )
+
+    render_and_check(
+        request=request,
+        mf_test_configuration=mf_test_configuration,
+        dataflow_to_sql_converter=dataflow_to_sql_converter,
+        sql_client=sql_client,
+        dataflow_plan_builder=dataflow_plan_builder,
+        query_spec=query_spec,
+    )