Fix time constraint adjustment

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

Hide file tree

Showing 4 changed files with 21 additions and 17 deletions.
diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/linkable_element_set.py b/metricflow-semantics/metricflow_semantics/model/semantics/linkable_element_set.py
@@ -386,7 +386,7 @@ def filter_by_spec_patterns(self, spec_patterns: Sequence[SpecPattern]) -> Linka
         """
         start_time = time.time()
 
-        # Spec patterns need all specs to match properly e.g. `DefaultTimeGranularityPattern`.
+        # Spec patterns need all specs to match properly e.g. `MinimumTimeGrainPattern`.
         matching_specs: Sequence[InstanceSpec] = self.specs
 
         for spec_pattern in spec_patterns:

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
@@ -28,7 +28,7 @@
     MetricFlowQueryResolutionIssueSet,
 )
 from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator, QueryPartForSuggestions
-from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern
+from metricflow_semantics.specs.patterns.base_time_grain import MinimumTimeGrainPattern
 from metricflow_semantics.specs.patterns.default_time_granularity import DefaultTimeGranularityPattern
 from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern
 from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern
@@ -83,11 +83,14 @@ def resolve_matching_item_for_querying(
         spec_pattern: SpecPattern,
         suggestion_generator: Optional[QueryItemSuggestionGenerator],
         queried_metrics: Sequence[MetricReference],
+        use_minimum_grain: bool = False,
     ) -> GroupByItemResolution:
         """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, unless the spec is metric_time, in which case the default grain is returned.
+
+        If use_minimum_grain is True, will use minimum grain instead of default for metric_time, too.
         """
         push_down_visitor = _PushDownGroupByItemCandidatesVisitor(
             manifest_lookup=self._manifest_lookup,
@@ -104,11 +107,14 @@ def resolve_matching_item_for_querying(
                 issue_set=push_down_result.issue_set,
             )
 
-        push_down_result = push_down_result.filter_candidates_by_pattern(
-            DefaultTimeGranularityPattern(
+        filter_to_use = (
+            MinimumTimeGrainPattern()
+            if use_minimum_grain
+            else DefaultTimeGranularityPattern(
                 metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics
             )
         )
+        push_down_result = push_down_result.filter_candidates_by_pattern(filter_to_use)
 
         logger.info(
             f"Spec pattern:\n"
@@ -231,10 +237,11 @@ def resolve_default_metric_time_grain(self, metrics_in_query: Sequence[MetricRef
                 TimeDimensionCallParameterSet(
                     entity_path=(),
                     time_dimension_reference=TimeDimensionReference(element_name=METRIC_TIME_ELEMENT_NAME),
-                )
+                ),
             ),
             suggestion_generator=None,
             queried_metrics=metrics_in_query,
+            use_minimum_grain=True,
         )
         metric_time_spec_set = (
             group_specs_by_type((metric_time_grain_resolution.spec,))

diff --git a/metricflow-semantics/metricflow_semantics/query/query_parser.py b/metricflow-semantics/metricflow_semantics/query/query_parser.py
@@ -52,7 +52,7 @@
     ResolverInputForQuery,
     ResolverInputForQueryLevelWhereFilterIntersection,
 )
-from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern
+from metricflow_semantics.specs.patterns.base_time_grain import MinimumTimeGrainPattern
 from metricflow_semantics.specs.patterns.metric_time_pattern import MetricTimePattern
 from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern
 from metricflow_semantics.specs.query_param_implementations import DimensionOrEntityParameter, MetricParameter
@@ -154,7 +154,7 @@ def _get_smallest_requested_metric_time_granularity(
 
         for pattern_to_apply in (
             MetricTimePattern(),
-            BaseTimeGrainPattern(),
+            MinimumTimeGrainPattern(),
             NoneDatePartPattern(),
         ):
             matching_specs = pattern_to_apply.match(matching_specs)
@@ -165,7 +165,7 @@ def _get_smallest_requested_metric_time_granularity(
 
         assert (
             len(time_dimension_specs) == 1
-        ), f"Bug with DefaultTimeGranularityPattern - should have returned exactly 1 spec but got {time_dimension_specs}"
+        ), f"Bug with MinimumTimeGrainPattern - should have returned exactly 1 spec but got {time_dimension_specs}"
 
         return time_dimension_specs[0].time_granularity
 
@@ -182,8 +182,7 @@ def _adjust_time_constraint(
         """
         metric_time_granularity = self._get_smallest_requested_metric_time_granularity(time_dimension_specs_in_query)
         if metric_time_granularity is None:
-            # This indicates there were no metric time specs in the query with granularity specified, so use
-            # the queried metrics to figure out what the granularity will be resolved to.
+            # This indicates there were no metric time specs in the query, so use smallest available granularity for metric_time.
             group_by_item_resolver = GroupByItemResolver(
                 manifest_lookup=self._manifest_lookup,
                 resolution_dag=resolution_dag,
@@ -507,11 +506,7 @@ def _parse_and_validate_query(
                 ),
             )
             logger.info(f"Time constraint after adjustment is: {time_constraint}")
-
-            return ParseQueryResult(
-                query_spec=query_spec.with_time_range_constraint(time_constraint),
-                queried_semantic_models=query_resolution.queried_semantic_models,
-            )
+            query_spec = query_spec.with_time_range_constraint(time_constraint)
 
         return ParseQueryResult(
             query_spec=query_spec,

diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py b/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py
@@ -18,7 +18,7 @@
 from metricflow_semantics.specs.spec_set import group_specs_by_type
 
 
-class BaseTimeGrainPattern(SpecPattern):
+class MinimumTimeGrainPattern(SpecPattern):
     """A pattern that matches linkable specs, but for time dimension specs, only the one with the finest grain.
 
     e.g.
@@ -63,7 +63,9 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe
             metric_time_specs = MetricTimePattern().match(candidate_specs)
             other_specs = tuple(spec for spec in candidate_specs if spec not in metric_time_specs)
 
-            return other_specs + tuple(BaseTimeGrainPattern(only_apply_for_metric_time=False).match(metric_time_specs))
+            return other_specs + tuple(
+                MinimumTimeGrainPattern(only_apply_for_metric_time=False).match(metric_time_specs)
+            )
 
         spec_set = group_specs_by_type(candidate_specs)