Debug & get working!

dbt-labs · Jul 10, 2024 · adb5274 · adb5274
1 parent 906fd0a
commit adb5274
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 9 deletions.
diff --git a/metricflow-semantics/metricflow_semantics/query/group_by_item/group_by_item_resolver.py b/metricflow-semantics/metricflow_semantics/query/group_by_item/group_by_item_resolver.py
@@ -87,7 +87,7 @@ def resolve_matching_item_for_querying(
         """Returns the spec that corresponds to the one described by spec_pattern and is valid for the query.
 
         For queries, if the pattern matches to a spec for the same element at different grains, the spec with the finest
-        common grain is returned.
+        common grain is returned, unless the spec is metric_time, in which case the default grain is returned.
         """
         push_down_visitor = _PushDownGroupByItemCandidatesVisitor(
             manifest_lookup=self._manifest_lookup,
@@ -104,13 +104,12 @@ def resolve_matching_item_for_querying(
                 issue_set=push_down_result.issue_set,
             )
 
-        # TODO: this isn't working, figure out why
-        # filter_to_use = DefaultTimeGranularityPattern(
-        #     metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics
-        # )
         push_down_result = push_down_result.filter_candidates_by_pattern(
-            BaseTimeGrainPattern(),
+            DefaultTimeGranularityPattern(
+                metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics
+            )
         )
+
         logger.info(
             f"Spec pattern:\n"
             f"{indent(mf_pformat(spec_pattern))}\n"
@@ -130,7 +129,6 @@ def resolve_matching_item_for_querying(
                     )
                 ),
             )
-        # TODO: return issue if push_down_result.candidate_set.num_candidates < 1
 
         return GroupByItemResolution(
             spec=push_down_result.candidate_set.specs[0],

diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py
@@ -77,7 +77,14 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe
         for spec_key, time_grains in spec_key_to_grains.items():
             grain_to_use = (
                 default_granularity_for_metrics
-                if (default_granularity_for_metrics and spec_key.source_spec.is_metric_time)
+                if (
+                    default_granularity_for_metrics
+                    # Use default_granularity for metric_time, but minimum granularity for all other time dims.
+                    and spec_key.source_spec.is_metric_time
+                    # If default_granularity is not in the available options, then time granularity was probably
+                    # specified in the request and does not need a default here.
+                    and default_granularity_for_metrics in time_grains
+                )
                 else min(time_grains)
             )
             matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(grain_to_use))

diff --git a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py
@@ -685,3 +685,10 @@ def test_default_granularity(bookings_query_parser: MetricFlowQueryParser) -> No
     ).query_spec
     assert len(query_spec.time_dimension_specs) == 1
     assert query_spec.time_dimension_specs[0] == MTD_SPEC_MONTH
+
+    # Using non-metric_time - should ignore default granularity
+    query_spec = bookings_query_parser.parse_and_validate_query(
+        metric_names=("instant_bookings",), group_by_names=("booking__ds",)
+    ).query_spec
+    assert len(query_spec.time_dimension_specs) == 1
+    assert query_spec.time_dimension_specs[0].time_granularity == TimeGranularity.DAY
diff --git a/tests_metricflow/integration/test_cases/itest_measure_aggregations.yaml b/tests_metricflow/integration/test_cases/itest_measure_aggregations.yaml
@@ -8,7 +8,7 @@ integration_test:
   check_query: |
     SELECT
       MAX(capacity) as largest_listing
-      , created_at AS metric_time__day
+      , {{ render_date_trunc("created_at", TimeGranularity.MONTH) }} AS metric_time__month
     FROM {{ source_schema }}.dim_listings_latest
     GROUP BY
       2