Remove base time grain restriction for cumulative metrics

dbt-labs · Jun 19, 2024 · 55ecd79 · 55ecd79
1 parent 02b8f69
commit 55ecd79
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 13 deletions.
diff --git a/...mantics/metricflow_semantics/query/group_by_item/candidate_push_down/push_down_visitor.py b/...mantics/metricflow_semantics/query/group_by_item/candidate_push_down/push_down_visitor.py
@@ -46,7 +46,6 @@
     MetricFlowQueryResolutionIssueSet,
 )
 from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator
-from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern
 from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern
 from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern
 
@@ -171,19 +170,8 @@ def visit_measure_node(self, node: MeasureGroupByItemSourceNode) -> PushDownResu
             elif metric.type is MetricType.RATIO or metric.type is MetricType.DERIVED:
                 assert False, f"A measure should have a simple or cumulative metric as a child, but got {metric.type}"
             elif metric.type is MetricType.CUMULATIVE:
-                # To handle the restriction that cumulative metrics can only be queried at the base grain, it's
-                # easiest to handle that by applying the pattern to remove non-base grain time dimension specs at the
-                # measure node and generate the issue here if there's nothing that matches. Generating the issue here
-                # allows for creation of a more specific issue (i.e. include the measure) vs. generating the issue
-                # at a higher level. This can be more cleanly handled once we add additional context to the
-                # LinkableInstanceSpec.
                 patterns_to_apply = (
-                    # From comment in ValidLinkableSpecResolver:
-                    #   It's possible to aggregate measures to coarser time granularities
-                    #   (except with cumulative metrics).
-                    BaseTimeGrainPattern(only_apply_for_metric_time=True),
-                    # From comment in previous query parser:
-                    #   Cannot extract date part for cumulative metrics.
+                    # Date part doesn't make clear sense with cumulative metrics, so we don't allow it.
                     NoneDatePartPattern(),
                 )
             else:

diff --git a/.../LinkableSpecSet/test_available_group_by_items__accumulate_last_2_months_metric__set0.txt b/.../LinkableSpecSet/test_available_group_by_items__accumulate_last_2_months_metric__set0.txt
@@ -1,5 +1,7 @@
 [
   "TimeDimension('metric_time', 'month')",
+  "TimeDimension('metric_time', 'quarter')",
+  "TimeDimension('metric_time', 'year')",
   "TimeDimension('monthly_measure_entity__creation_time', 'month')",
   "TimeDimension('monthly_measure_entity__creation_time', 'quarter')",
   "TimeDimension('monthly_measure_entity__creation_time', 'year')",