diff --git a/metricflow-semantics/metricflow_semantics/query/group_by_item/candidate_push_down/push_down_visitor.py b/metricflow-semantics/metricflow_semantics/query/group_by_item/candidate_push_down/push_down_visitor.py index 2256db8a60..de488ad68b 100644 --- a/metricflow-semantics/metricflow_semantics/query/group_by_item/candidate_push_down/push_down_visitor.py +++ b/metricflow-semantics/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/metricflow-semantics/tests_metricflow_semantics/snapshots/test_available_group_by_items.py/LinkableSpecSet/test_available_group_by_items__accumulate_last_2_months_metric__set0.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_available_group_by_items.py/LinkableSpecSet/test_available_group_by_items__accumulate_last_2_months_metric__set0.txt index 5471827922..6d8275d0c1 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_available_group_by_items.py/LinkableSpecSet/test_available_group_by_items__accumulate_last_2_months_metric__set0.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_available_group_by_items.py/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')",