diff --git a/.changes/unreleased/Features-20240614-071108.yaml b/.changes/unreleased/Features-20240614-071108.yaml new file mode 100644 index 0000000000..7b6e1a8eac --- /dev/null +++ b/.changes/unreleased/Features-20240614-071108.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Remove restriction on querying non-default granularities with cumulative metrics. +time: 2024-06-14T07:11:08.765605-07:00 +custom: + Author: courtneyholcomb + Issue: "1282" 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..4330579e26 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: @@ -198,7 +186,7 @@ def visit_measure_node(self, node: MeasureGroupByItemSourceNode) -> PushDownResu f"For {node.ui_description}:\n" + indent( "After applying patterns:\n" - + indent(mf_pformat(patterns_to_apply)) + + indent(mf_pformat(patterns_to_apply + self._source_spec_patterns)) + "\n" + "to inputs, matches are:\n" + indent(mf_pformat(matching_items.specs)) 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')",