diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/linkable_element_set.py b/metricflow-semantics/metricflow_semantics/model/semantics/linkable_element_set.py index 2664abf3f3..378719f45f 100644 --- 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 index 32c48b9ae5..40afd31a78 100644 --- 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 index 7d29286752..56078e32e0 100644 --- 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 index 34b44c1267..dcd1a930ce 100644 --- 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) diff --git a/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/metrics.yaml b/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/metrics.yaml index ce1854fd06..1f815fa52d 100644 --- a/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/metrics.yaml +++ b/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 diff --git a/tests_metricflow/integration/query_output/test_fill_nulls_with_0.py b/tests_metricflow/integration/query_output/test_fill_nulls_with_0.py index 3847b3d572..d9b05f6b00 100644 --- a/tests_metricflow/integration/query_output/test_fill_nulls_with_0.py +++ b/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), ) diff --git a/tests_metricflow/integration/test_cases/itest_metrics.yaml b/tests_metricflow/integration/test_cases/itest_metrics.yaml index 54006e9be4..219838e8cd 100644 --- a/tests_metricflow/integration/test_cases/itest_metrics.yaml +++ b/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 diff --git a/tests_metricflow/query_rendering/test_fill_nulls_with_rendering.py b/tests_metricflow/query_rendering/test_fill_nulls_with_rendering.py index ee8b868453..9feab48212 100644 --- a/tests_metricflow/query_rendering/test_fill_nulls_with_rendering.py +++ b/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), diff --git a/tests_metricflow/snapshots/test_fill_nulls_with_0.py/str/DuckDB/test_fill_nulls_with_0_multi_metric_query__query_output.txt b/tests_metricflow/snapshots/test_fill_nulls_with_0.py/str/DuckDB/test_fill_nulls_with_0_multi_metric_query__query_output.txt index c70bb40bf0..10e1ce0068 100644 --- a/tests_metricflow/snapshots/test_fill_nulls_with_0.py/str/DuckDB/test_fill_nulls_with_0_multi_metric_query__query_output.txt +++ b/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