Fix time constraint adjustment

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:

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,))

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,

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)
 

metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/metrics.yaml

@@ -627,6 +627,7 @@ metric:
       name: bookings
       join_to_timespine: true
       fill_nulls_with: 0
+  default_granularity: week
 ---
 metric:
   name: every_two_days_bookers_fill_nulls_with_0

tests_metricflow/integration/query_output/test_fill_nulls_with_0.py

@@ -22,8 +22,8 @@ def test_simple_fill_nulls_with_0_metric_time(  # noqa: D103
     query_result = it_helpers.mf_engine.query(
         MetricFlowQueryRequest.create_with_random_request_id(
             metric_names=["bookings_fill_nulls_with_0"],
-            group_by_names=["metric_time"],
-            order_by_names=["metric_time"],
+            group_by_names=["metric_time__day"],
+            order_by_names=["metric_time__day"],
             time_constraint_start=datetime.datetime(2019, 11, 27),
             time_constraint_end=datetime.datetime(2020, 1, 5),
         )

tests_metricflow/integration/test_cases/itest_metrics.yaml

@@ -1140,7 +1140,7 @@ integration_test:
   description: Test a simple query that joins to time spine and fills nulls
   model: SIMPLE_MODEL
   metrics: ["bookings_fill_nulls_with_0"]
-  group_by_objs: [{"name": "metric_time"}]
+  group_by_objs: [{"name": "metric_time__day"}]
   check_query: |
     SELECT
       subq_5.ds AS metric_time__day
@@ -1340,7 +1340,7 @@ integration_test:
   description: Test a multi-metric query that fills nulls
   model: SIMPLE_MODEL
   metrics: ["bookings_fill_nulls_with_0", "views"]
-  group_by_objs: [{"name": "metric_time"}]
+  group_by_objs: [{"name": "metric_time__day"}]
   check_query: |
     SELECT
       COALESCE(subq_7.metric_time__day, subq_12.metric_time__day) AS metric_time__day

tests_metricflow/query_rendering/test_fill_nulls_with_rendering.py

@@ -201,7 +201,7 @@ def test_join_to_time_spine_with_filters(  # noqa: D103
         metric_names=("bookings_fill_nulls_with_0",),
         group_by_names=("metric_time__day",),
         where_constraint=PydanticWhereFilter(
-            where_sql_template="{{ TimeDimension('metric_time') }} > '2020-01-01'",
+            where_sql_template="{{ TimeDimension('metric_time', 'day') }} > '2020-01-01'",
         ),
         time_constraint_start=datetime.datetime(2020, 1, 3),
         time_constraint_end=datetime.datetime(2020, 1, 5),

tests_metricflow/snapshots/test_fill_nulls_with_0.py/str/DuckDB/test_fill_nulls_with_0_multi_metric_query__query_output.txt

@@ -1,42 +1,8 @@
-metric_time__day       bookings_fill_nulls_with_0  views
+metric_time__week      bookings_fill_nulls_with_0  views
 -------------------  ----------------------------  -------
-2019-11-27T00:00:00                             0  None
-2019-11-28T00:00:00                             0  None
-2019-11-29T00:00:00                             0  None
-2019-11-30T00:00:00                             0  None
-2019-12-01T00:00:00                             1  None
+2019-11-25T00:00:00                             1  None
 2019-12-02T00:00:00                             0  None
-2019-12-03T00:00:00                             0  None
-2019-12-04T00:00:00                             0  None
-2019-12-05T00:00:00                             0  None
-2019-12-06T00:00:00                             0  None
-2019-12-07T00:00:00                             0  None
-2019-12-08T00:00:00                             0  None
 2019-12-09T00:00:00                             0  None
-2019-12-10T00:00:00                             0  None
-2019-12-11T00:00:00                             0  None
-2019-12-12T00:00:00                             0  None
-2019-12-13T00:00:00                             0  None
-2019-12-14T00:00:00                             0  None
-2019-12-15T00:00:00                             0  None
-2019-12-16T00:00:00                             0  None
-2019-12-17T00:00:00                             0  None
-2019-12-18T00:00:00                            10  None
-2019-12-19T00:00:00                            18  None
-2019-12-20T00:00:00                             2  None
-2019-12-21T00:00:00                             0  None
-2019-12-22T00:00:00                             0  None
+2019-12-16T00:00:00                            30  None
 2019-12-23T00:00:00                             0  None
-2019-12-24T00:00:00                             0  None
-2019-12-25T00:00:00                             0  None
-2019-12-26T00:00:00                             0  None
-2019-12-27T00:00:00                             0  None
-2019-12-28T00:00:00                             0  None
-2019-12-29T00:00:00                             0  None
-2019-12-30T00:00:00                             0  None
-2019-12-31T00:00:00                             0  None
-2020-01-01T00:00:00                             5  2
-2020-01-02T00:00:00                             9  5
-2020-01-03T00:00:00                             1  None
-2020-01-04T00:00:00                             0  1
-2020-01-05T00:00:00                             0  1
+2019-12-30T00:00:00                            15  9