From 791e1c96b7718e4fb289098d6ff2eae59a8dd052 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Fri, 28 Jun 2024 09:19:57 -0700 Subject: [PATCH 01/20] Bump DSI dev version to get default grain specs --- extra-hatch-configuration/requirements.txt | 2 +- .../model/semantics/semantic_model_lookup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extra-hatch-configuration/requirements.txt b/extra-hatch-configuration/requirements.txt index db1991c6ae..2bd42b50ec 100644 --- a/extra-hatch-configuration/requirements.txt +++ b/extra-hatch-configuration/requirements.txt @@ -1,5 +1,5 @@ Jinja2>=3.1.3 -dbt-semantic-interfaces==0.6.1 +dbt-semantic-interfaces==0.6.2.dev2 more-itertools>=8.10.0, <10.2.0 pydantic>=1.10.0, <3.0 tabulate>=0.8.9 diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py b/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py index d820efb9a0..0fbc1b0c21 100644 --- a/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py +++ b/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py @@ -104,7 +104,7 @@ def get_dimension(self, dimension_reference: DimensionReference) -> Dimension: def get_time_dimension(self, time_dimension_reference: TimeDimensionReference) -> Dimension: """Retrieves a full dimension object by name.""" - return self.get_dimension(dimension_reference=time_dimension_reference.dimension_reference()) + return self.get_dimension(dimension_reference=time_dimension_reference.dimension_reference) @property def measure_references(self) -> Sequence[MeasureReference]: From 64b06da0dec7b5bb5daaef9bc5810373774a5078 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Fri, 28 Jun 2024 09:17:00 -0700 Subject: [PATCH 02/20] Changelog --- .changes/unreleased/Features-20240628-074617.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20240628-074617.yaml diff --git a/.changes/unreleased/Features-20240628-074617.yaml b/.changes/unreleased/Features-20240628-074617.yaml new file mode 100644 index 0000000000..f3bbcc8e6f --- /dev/null +++ b/.changes/unreleased/Features-20240628-074617.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Use default_grain to resolve metric_time and cumulative metric queries. +time: 2024-06-28T07:46:17.768805-07:00 +custom: + Author: courtneyholcomb + Issue: "1310" From 142c0b13310a63b51bd4f2e5aa62c488720658e2 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Fri, 28 Jun 2024 07:44:10 -0700 Subject: [PATCH 03/20] Apply SetDefaultGranularityRule to all manifests --- .../metricflow_semantics/model/dbt_manifest_parser.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/metricflow-semantics/metricflow_semantics/model/dbt_manifest_parser.py b/metricflow-semantics/metricflow_semantics/model/dbt_manifest_parser.py index 638da5283f..903ac85fb5 100644 --- a/metricflow-semantics/metricflow_semantics/model/dbt_manifest_parser.py +++ b/metricflow-semantics/metricflow_semantics/model/dbt_manifest_parser.py @@ -11,6 +11,7 @@ ConvertMedianToPercentileRule, ) from dbt_semantic_interfaces.transformations.cumulative_type_params import SetCumulativeTypeParamsRule +from dbt_semantic_interfaces.transformations.default_granularity import SetDefaultGranularityRule from dbt_semantic_interfaces.transformations.names import LowerCaseNamesRule from dbt_semantic_interfaces.transformations.proxy_measure import CreateProxyMeasureRule from dbt_semantic_interfaces.transformations.semantic_manifest_transformer import ( @@ -38,6 +39,7 @@ def parse_manifest_from_dbt_generated_manifest(manifest_json_string: str) -> Pyd ConvertMedianToPercentileRule(), DedupeMetricInputMeasuresRule(), # Remove once fix is in core SetCumulativeTypeParamsRule(), + SetDefaultGranularityRule(), ), ) model = PydanticSemanticManifestTransformer.transform(raw_model, rules) From 23b628fc86f10ad9e63d7c9bf979f91c7f73a228 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Fri, 28 Jun 2024 11:41:02 -0700 Subject: [PATCH 04/20] WIP --- .../metricflow_semantics/model/semantics/metric_lookup.py | 2 ++ .../metricflow_semantics/specs/spec_classes.py | 8 +++++++- metricflow/dataflow/builder/dataflow_plan_builder.py | 7 +++++-- metricflow/plan_conversion/dataflow_to_sql.py | 2 +- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py b/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py index 21e5fb8ac1..60973462a9 100644 --- a/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py +++ b/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py @@ -120,6 +120,8 @@ def get_metric(self, metric_reference: MetricReference) -> Metric: # noqa: D102 def _add_metric(self, metric: Metric) -> None: """Add metric, validating presence of required measures.""" + print("attributes??", dir(metric), type(metric)) + assert metric.default_granularity, "whyyy" metric_reference = MetricReference(element_name=metric.name) if metric_reference in self._metrics: raise DuplicateMetricError(f"Metric `{metric.name}` has already been registered") diff --git a/metricflow-semantics/metricflow_semantics/specs/spec_classes.py b/metricflow-semantics/metricflow_semantics/specs/spec_classes.py index 546f07f58d..ebfdeb7c1c 100644 --- a/metricflow-semantics/metricflow_semantics/specs/spec_classes.py +++ b/metricflow-semantics/metricflow_semantics/specs/spec_classes.py @@ -570,6 +570,7 @@ class MetricSpec(InstanceSpec): # noqa: D101 alias: Optional[str] = None offset_window: Optional[PydanticMetricTimeWindow] = None offset_to_grain: Optional[TimeGranularity] = None + default_granularity: Optional[TimeGranularity] = None @staticmethod def from_element_name(element_name: str) -> MetricSpec: # noqa: D102 @@ -598,7 +599,12 @@ def has_time_offset(self) -> bool: # noqa: D102 def without_offset(self) -> MetricSpec: """Represents the metric spec with any time offsets removed.""" - return MetricSpec(element_name=self.element_name, filter_specs=self.filter_specs, alias=self.alias) + return MetricSpec( + element_name=self.element_name, + filter_specs=self.filter_specs, + alias=self.alias, + default_granularity=self.default_granularity, + ) @dataclass(frozen=True) diff --git a/metricflow/dataflow/builder/dataflow_plan_builder.py b/metricflow/dataflow/builder/dataflow_plan_builder.py index 096d63edbf..a643628d7b 100644 --- a/metricflow/dataflow/builder/dataflow_plan_builder.py +++ b/metricflow/dataflow/builder/dataflow_plan_builder.py @@ -432,8 +432,11 @@ def _build_cumulative_metric_output_node( predicate_pushdown_state: PredicatePushdownState, for_group_by_source_node: bool = False, ) -> DataflowPlanNode: - # TODO: replace with default_grain once added to YAML spec - default_granularity = self._metric_lookup.get_min_queryable_time_granularity(metric_spec.reference) + # What is the expected behavior if you query with default grain? + # What if you query with a grain smaller than default? And larger? + # TODO elsewhere: use default grain for metric_time resolution + default_granularity = metric_spec.default_granularity + assert default_granularity, f"No default_granularity set for {metric_spec}. Something has been misconfigured." queried_agg_time_dimensions = queried_linkable_specs.included_agg_time_dimension_specs_for_metric( metric_reference=metric_spec.reference, metric_lookup=self._metric_lookup diff --git a/metricflow/plan_conversion/dataflow_to_sql.py b/metricflow/plan_conversion/dataflow_to_sql.py index c3ad1d542f..65c6ddb1b0 100644 --- a/metricflow/plan_conversion/dataflow_to_sql.py +++ b/metricflow/plan_conversion/dataflow_to_sql.py @@ -297,7 +297,7 @@ def visit_join_over_time_range_node(self, node: JoinOverTimeRangeNode) -> SqlDat # Find requested agg_time_dimensions in parent instance set. # For now, will use instance with smallest granularity in time spine join. - # TODO: use metric's default_grain once that property is available. + # TODO: use metric's default_granularity once that property is available. agg_time_dimension_instance_for_join: Optional[TimeDimensionInstance] = None requested_agg_time_dimension_instances: Tuple[TimeDimensionInstance, ...] = () for instance in input_data_set.instance_set.time_dimension_instances: From 9f0dcbed82b8135a591462d40b04e4adb134f373 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Mon, 1 Jul 2024 16:01:59 -0700 Subject: [PATCH 05/20] Bump DSI version to get default_granularity spec changes --- extra-hatch-configuration/requirements.txt | 2 +- metricflow-semantics/extra-hatch-configuration/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extra-hatch-configuration/requirements.txt b/extra-hatch-configuration/requirements.txt index 2bd42b50ec..ff7374aba3 100644 --- a/extra-hatch-configuration/requirements.txt +++ b/extra-hatch-configuration/requirements.txt @@ -1,5 +1,5 @@ Jinja2>=3.1.3 -dbt-semantic-interfaces==0.6.2.dev2 +dbt-semantic-interfaces>=0.6.2 more-itertools>=8.10.0, <10.2.0 pydantic>=1.10.0, <3.0 tabulate>=0.8.9 diff --git a/metricflow-semantics/extra-hatch-configuration/requirements.txt b/metricflow-semantics/extra-hatch-configuration/requirements.txt index 6131ae86f3..a69f99c67e 100644 --- a/metricflow-semantics/extra-hatch-configuration/requirements.txt +++ b/metricflow-semantics/extra-hatch-configuration/requirements.txt @@ -1,5 +1,5 @@ # dbt Cloud depends on metricflow-semantics (dependency set in dbt-mantle), so DSI must always point to a production version here. -dbt-semantic-interfaces>=0.6.1, <2.0.0 +dbt-semantic-interfaces>=0.6.2 graphviz>=0.18.2, <0.21 python-dateutil>=2.9.0, <2.10.0 rapidfuzz>=3.0, <4.0 From b3756bc1655542a1517403dcf59e6a714e4abcc2 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Mon, 1 Jul 2024 16:30:24 -0700 Subject: [PATCH 06/20] Rename BaseTimeGrainPattern .-> DefaultGranularityPattern and update documentation --- .../model/semantics/linkable_element_set.py | 2 +- .../query/group_by_item/group_by_item_resolver.py | 6 +++--- .../metricflow_semantics/query/query_parser.py | 6 +++--- .../metricflow_semantics/query/suggestion_generator.py | 6 +++--- .../metricflow_semantics/specs/patterns/base_time_grain.py | 4 ++-- .../specs/query_param_implementations.py | 1 + 6 files changed, 13 insertions(+), 12 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 index effa5daf92..2664abf3f3 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. `BaseTimeGrainPattern`. + # Spec patterns need all specs to match properly e.g. `DefaultTimeGranularityPattern`. 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 dbc861467e..0671d94e73 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 @@ -27,7 +27,7 @@ MetricFlowQueryResolutionIssueSet, ) from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator -from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern +from metricflow_semantics.specs.patterns.base_time_grain import DefaultTimeGranularityPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern from metricflow_semantics.specs.patterns.typed_patterns import TimeDimensionPattern @@ -102,7 +102,7 @@ def resolve_matching_item_for_querying( ) push_down_result = push_down_result.filter_candidates_by_pattern( - BaseTimeGrainPattern(), + DefaultTimeGranularityPattern(), ) logger.info( f"Spec pattern:\n" @@ -152,7 +152,7 @@ def resolve_matching_item_for_filters( push_down_visitor = _PushDownGroupByItemCandidatesVisitor( manifest_lookup=self._manifest_lookup, - source_spec_patterns=(spec_pattern, BaseTimeGrainPattern()), + source_spec_patterns=(spec_pattern, DefaultTimeGranularityPattern()), suggestion_generator=suggestion_generator, ) diff --git a/metricflow-semantics/metricflow_semantics/query/query_parser.py b/metricflow-semantics/metricflow_semantics/query/query_parser.py index f6177ca01d..4b399b9ac4 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 DefaultTimeGranularityPattern 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 @@ -153,7 +153,7 @@ def _metric_time_granularity(time_dimension_specs: Sequence[TimeDimensionSpec]) for pattern_to_apply in ( MetricTimePattern(), - BaseTimeGrainPattern(), + DefaultTimeGranularityPattern(), NoneDatePartPattern(), ): matching_specs = pattern_to_apply.match(matching_specs) @@ -164,7 +164,7 @@ def _metric_time_granularity(time_dimension_specs: Sequence[TimeDimensionSpec]) assert ( len(time_dimension_specs) == 1 - ), f"Bug with BaseTimeGrainPattern - should have returned exactly 1 spec but got {time_dimension_specs}" + ), f"Bug with DefaultTimeGranularityPattern - should have returned exactly 1 spec but got {time_dimension_specs}" return time_dimension_specs[0].time_granularity diff --git a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py index ce53c50b74..ba7b59af56 100644 --- a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py +++ b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py @@ -5,7 +5,7 @@ from metricflow_semantics.naming.naming_scheme import QueryItemNamingScheme from metricflow_semantics.query.similarity import top_fuzzy_matches -from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern +from metricflow_semantics.specs.patterns.base_time_grain import DefaultTimeGranularityPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern @@ -24,9 +24,9 @@ class QueryItemSuggestionGenerator: # Adding these filters so that we don't get multiple suggestions that are similar, but with different # grains. Some additional thought is needed to tweak this as the base grain may not be the best suggestion. - FILTER_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = (BaseTimeGrainPattern(), NoneDatePartPattern()) + FILTER_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = (DefaultTimeGranularityPattern(), NoneDatePartPattern()) GROUP_BY_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = ( - BaseTimeGrainPattern(), + DefaultTimeGranularityPattern(), NoneDatePartPattern(), NoGroupByMetricPattern(), ) 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..c9a779331c 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 DefaultTimeGranularityPattern(SpecPattern): """A pattern that matches linkable specs, but for time dimension specs, only the one with the finest grain. e.g. @@ -63,7 +63,7 @@ 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(DefaultTimeGranularityPattern().match(metric_time_specs)) spec_set = group_specs_by_type(candidate_specs) diff --git a/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py b/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py index 697b65db8f..4e2c32da35 100644 --- a/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py +++ b/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py @@ -53,6 +53,7 @@ def query_resolver_input(self) -> ResolverInputForGroupByItem: # noqa: D102 ParameterSetField.ENTITY_LINKS, ParameterSetField.DATE_PART, ] + # Is this left out because it needs to be resplved? If so document that. if self.grain is not None: fields_to_compare.append(ParameterSetField.TIME_GRANULARITY) From 10c4007f45bc4743f53f4ec5a118e10f6f3be5f2 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Tue, 2 Jul 2024 07:24:02 -0700 Subject: [PATCH 07/20] Remove anything related to using default_granularity for cumulative metrics --- .../unreleased/Features-20240628-074617.yaml | 2 +- .../model/semantics/metric_lookup.py | 2 - .../specs/patterns/base_time_grain.py | 37 +++++++------------ .../dataflow/builder/dataflow_plan_builder.py | 6 +-- metricflow/plan_conversion/dataflow_to_sql.py | 3 +- 5 files changed, 16 insertions(+), 34 deletions(-) diff --git a/.changes/unreleased/Features-20240628-074617.yaml b/.changes/unreleased/Features-20240628-074617.yaml index f3bbcc8e6f..2ad17eaf7d 100644 --- a/.changes/unreleased/Features-20240628-074617.yaml +++ b/.changes/unreleased/Features-20240628-074617.yaml @@ -1,5 +1,5 @@ kind: Features -body: Use default_grain to resolve metric_time and cumulative metric queries. +body: Use default_grain to resolve metric_time. time: 2024-06-28T07:46:17.768805-07:00 custom: Author: courtneyholcomb diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py b/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py index 60973462a9..21e5fb8ac1 100644 --- a/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py +++ b/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py @@ -120,8 +120,6 @@ def get_metric(self, metric_reference: MetricReference) -> Metric: # noqa: D102 def _add_metric(self, metric: Metric) -> None: """Add metric, validating presence of required measures.""" - print("attributes??", dir(metric), type(metric)) - assert metric.default_granularity, "whyyy" metric_reference = MetricReference(element_name=metric.name) if metric_reference in self._metrics: raise DuplicateMetricError(f"Metric `{metric.name}` has already been registered") 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 c9a779331c..396242ce70 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py @@ -6,7 +6,6 @@ from dbt_semantic_interfaces.type_enums import TimeGranularity from typing_extensions import override -from metricflow_semantics.specs.patterns.metric_time_pattern import MetricTimePattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern from metricflow_semantics.specs.spec_classes import ( InstanceSpec, @@ -19,7 +18,14 @@ class DefaultTimeGranularityPattern(SpecPattern): - """A pattern that matches linkable specs, but for time dimension specs, only the one with the finest grain. + """A pattern that matches linkable specs, but for time dimension specs, only the one with the default granularity. + + When queried with metrics, default_granularity is specified in the YAML spec for the metrics. If this field is not + set for any of the queried metric(s), defaults to DAY for those metrics unless the minimum granularity is larger than + DAY. In that case, defaults to the smallest available granularity. + + Same defaults apply if no metrics are queried: default to DAY if available for the time dimension queried, else the + smallest available granularity. Always defaults to DAY for metric_time if not metrics are queried. e.g. @@ -39,32 +45,12 @@ class DefaultTimeGranularityPattern(SpecPattern): The finest grain represents the defined grain of the time dimension in the semantic model when evaluating specs of the source. - This pattern helps to implement matching of group-by-items for where filters - in those cases, an ambiguously - specified group-by-item can only match to time dimension spec with the base grain. - - Also, this is currently used to help implement restrictions on cumulative metrics where they can only be queried - by the base grain of metric_time. + This pattern helps to implement matching of group-by-items. An ambiguously specified group-by-item can only match to + time dimension spec with the base grain. """ - def __init__(self, only_apply_for_metric_time: bool = False) -> None: - """Initializer. - - Args: - only_apply_for_metric_time: If set, only remove time dimension specs with a non-base grain if it's for - metric_time. This parameter is useful for implementing restrictions on cumulative metrics as they can only - be queried by the base grain of metric_time. - TODO: This is a little odd. This can be replaced once composite patterns are supported. - """ - self._only_apply_for_metric_time = only_apply_for_metric_time - @override def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpec]: - if self._only_apply_for_metric_time: - 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(DefaultTimeGranularityPattern().match(metric_time_specs)) - spec_set = group_specs_by_type(candidate_specs) spec_key_to_grains: Dict[TimeDimensionSpecComparisonKey, Set[TimeGranularity]] = defaultdict(set) @@ -76,6 +62,9 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe matched_time_dimension_specs: List[TimeDimensionSpec] = [] for spec_key, time_grains in spec_key_to_grains.items(): + # Replace this with new default logic! How does it know for metric time what grain is avail? + # Maybe that logic is done already when passed in here? + # But now we need separate logic if metrics are queried, so we'll need to pass in metrics here (optionally). matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(min(time_grains))) matching_specs: Sequence[LinkableInstanceSpec] = ( diff --git a/metricflow/dataflow/builder/dataflow_plan_builder.py b/metricflow/dataflow/builder/dataflow_plan_builder.py index a643628d7b..d9bee6b645 100644 --- a/metricflow/dataflow/builder/dataflow_plan_builder.py +++ b/metricflow/dataflow/builder/dataflow_plan_builder.py @@ -432,11 +432,7 @@ def _build_cumulative_metric_output_node( predicate_pushdown_state: PredicatePushdownState, for_group_by_source_node: bool = False, ) -> DataflowPlanNode: - # What is the expected behavior if you query with default grain? - # What if you query with a grain smaller than default? And larger? - # TODO elsewhere: use default grain for metric_time resolution - default_granularity = metric_spec.default_granularity - assert default_granularity, f"No default_granularity set for {metric_spec}. Something has been misconfigured." + default_granularity = self._metric_lookup.get_min_queryable_time_granularity(metric_spec.reference) queried_agg_time_dimensions = queried_linkable_specs.included_agg_time_dimension_specs_for_metric( metric_reference=metric_spec.reference, metric_lookup=self._metric_lookup diff --git a/metricflow/plan_conversion/dataflow_to_sql.py b/metricflow/plan_conversion/dataflow_to_sql.py index 65c6ddb1b0..623eaa568e 100644 --- a/metricflow/plan_conversion/dataflow_to_sql.py +++ b/metricflow/plan_conversion/dataflow_to_sql.py @@ -296,8 +296,7 @@ def visit_join_over_time_range_node(self, node: JoinOverTimeRangeNode) -> SqlDat input_data_set_alias = self._next_unique_table_alias() # Find requested agg_time_dimensions in parent instance set. - # For now, will use instance with smallest granularity in time spine join. - # TODO: use metric's default_granularity once that property is available. + # Will use instance with smallest granularity in time spine join. agg_time_dimension_instance_for_join: Optional[TimeDimensionInstance] = None requested_agg_time_dimension_instances: Tuple[TimeDimensionInstance, ...] = () for instance in input_data_set.instance_set.time_dimension_instances: From aac89e07da22a5629acf618e7e442aa691e4843c Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Tue, 2 Jul 2024 15:50:47 -0700 Subject: [PATCH 08/20] WIP --- .../model/semantics/metric_lookup.py | 15 ++ .../filter_spec_resolver.py | 1 + .../group_by_item/group_by_item_resolver.py | 31 +++- .../query/query_parser.py | 24 ++- .../query/query_resolver.py | 25 ++-- .../query/suggestion_generator.py | 81 ++++++++-- .../specs/patterns/base_time_grain.py | 53 ++++++- .../test_matching_item_for_filters.py | 2 + .../test_matching_item_for_querying.py | 5 +- ...tion_for_invalid_metric_filter__result.txt | 98 +------------ ...or_invalid_metric_input_filter__result.txt | 102 +------------ ...h_different_parent_time_grains__result.txt | 98 +------------ ...ics_with_different_time_grains__result.txt | 138 +++++------------- 13 files changed, 248 insertions(+), 425 deletions(-) diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py b/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py index 21e5fb8ac1..5b5f153595 100644 --- a/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py +++ b/metricflow-semantics/metricflow_semantics/model/semantics/metric_lookup.py @@ -210,3 +210,18 @@ def get_min_queryable_time_granularity(self, metric_reference: MetricReference) minimum_queryable_granularity = defined_time_granularity return minimum_queryable_granularity + + def get_default_granularity_for_metrics( + self, metric_references: Sequence[MetricReference] + ) -> Optional[TimeGranularity]: + """When querying a group of metrics, the default granularity will be the largest of the metrics' default granularities.""" + max_default_granularity: Optional[TimeGranularity] = None + for metric_reference in metric_references: + default_granularity = self.get_metric(metric_reference).default_granularity + assert ( + default_granularity + ), f"No default_granularity set for {metric_reference}. Something has been misconfigured." + if not max_default_granularity or default_granularity.to_int() > max_default_granularity.to_int(): + max_default_granularity = default_granularity + + return max_default_granularity diff --git a/metricflow-semantics/metricflow_semantics/query/group_by_item/filter_spec_resolution/filter_spec_resolver.py b/metricflow-semantics/metricflow_semantics/query/group_by_item/filter_spec_resolution/filter_spec_resolver.py index 0c3efcb5cc..989606f5f6 100644 --- a/metricflow-semantics/metricflow_semantics/query/group_by_item/filter_spec_resolution/filter_spec_resolver.py +++ b/metricflow-semantics/metricflow_semantics/query/group_by_item/filter_spec_resolution/filter_spec_resolver.py @@ -347,6 +347,7 @@ def _resolve_specs_for_where_filters( input_str=group_by_item_in_where_filter.object_builder_str, spec_pattern=group_by_item_in_where_filter.spec_pattern, resolution_node=current_node, + filter_location=filter_location, ) # The paths in the issue set are generated relative to the current node. For error messaging, it seems more # helpful for those paths to be relative to the query. To do, we have to add nodes from the resolution path. 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 0671d94e73..20724cddff 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 @@ -6,7 +6,7 @@ from dbt_semantic_interfaces.call_parameter_sets import TimeDimensionCallParameterSet from dbt_semantic_interfaces.naming.keywords import METRIC_TIME_ELEMENT_NAME -from dbt_semantic_interfaces.references import SemanticModelReference, TimeDimensionReference +from dbt_semantic_interfaces.references import MetricReference, SemanticModelReference, TimeDimensionReference from dbt_semantic_interfaces.type_enums import TimeGranularity from typing_extensions import override @@ -20,13 +20,14 @@ PushDownResult, _PushDownGroupByItemCandidatesVisitor, ) +from metricflow_semantics.query.group_by_item.filter_spec_resolution.filter_location import WhereFilterLocation from metricflow_semantics.query.group_by_item.resolution_dag.dag import GroupByItemResolutionDag, ResolutionDagSinkNode from metricflow_semantics.query.group_by_item.resolution_path import MetricFlowQueryResolutionPath from metricflow_semantics.query.issues.group_by_item_resolver.ambiguous_group_by_item import AmbiguousGroupByItemIssue from metricflow_semantics.query.issues.issues_base import ( MetricFlowQueryResolutionIssueSet, ) -from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator +from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator, QueryPartForSuggestions from metricflow_semantics.specs.patterns.base_time_grain import DefaultTimeGranularityPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern @@ -80,6 +81,7 @@ def resolve_matching_item_for_querying( self, spec_pattern: SpecPattern, suggestion_generator: Optional[QueryItemSuggestionGenerator], + queried_metrics: Sequence[MetricReference], ) -> GroupByItemResolution: """Returns the spec that corresponds the one described by spec_pattern and is valid for the query. @@ -102,7 +104,11 @@ def resolve_matching_item_for_querying( ) push_down_result = push_down_result.filter_candidates_by_pattern( - DefaultTimeGranularityPattern(), + DefaultTimeGranularityPattern( + metric_lookup=self._manifest_lookup.metric_lookup, + only_apply_for_metric_time=False, + queried_metrics=queried_metrics, + ), ) logger.info( f"Spec pattern:\n" @@ -135,6 +141,7 @@ def resolve_matching_item_for_filters( input_str: str, spec_pattern: SpecPattern, resolution_node: ResolutionDagSinkNode, + filter_location: WhereFilterLocation, ) -> GroupByItemResolution: """Returns the spec that matches the spec_pattern associated with the filter in the given node. @@ -147,12 +154,21 @@ def resolve_matching_item_for_filters( suggestion_generator = QueryItemSuggestionGenerator( input_naming_scheme=ObjectBuilderNamingScheme(), input_str=input_str, - candidate_filters=QueryItemSuggestionGenerator.FILTER_ITEM_CANDIDATE_FILTERS, + query_part=QueryPartForSuggestions.WHERE_FILTER, + metric_lookup=self._manifest_lookup.metric_lookup, + queried_metrics=filter_location.metric_references, ) push_down_visitor = _PushDownGroupByItemCandidatesVisitor( manifest_lookup=self._manifest_lookup, - source_spec_patterns=(spec_pattern, DefaultTimeGranularityPattern()), + source_spec_patterns=( + spec_pattern, + DefaultTimeGranularityPattern( + metric_lookup=self._manifest_lookup.metric_lookup, + only_apply_for_metric_time=False, + queried_metrics=filter_location.metric_references, + ), + ), suggestion_generator=suggestion_generator, ) @@ -210,8 +226,8 @@ def resolve_available_items( issue_set=push_down_result.issue_set, ) - def resolve_min_metric_time_grain(self) -> TimeGranularity: - """Returns the finest time grain of metric_time for querying.""" + def resolve_default_metric_time_grain(self, metrics_in_query: Sequence[MetricReference]) -> TimeGranularity: + """Returns the default time grain of metric_time for querying.""" metric_time_grain_resolution = self.resolve_matching_item_for_querying( spec_pattern=TimeDimensionPattern.from_call_parameter_set( TimeDimensionCallParameterSet( @@ -220,6 +236,7 @@ def resolve_min_metric_time_grain(self) -> TimeGranularity: ) ), suggestion_generator=None, + queried_metrics=metrics_in_query, ) 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 4b399b9ac4..8edfe163e5 100644 --- a/metricflow-semantics/metricflow_semantics/query/query_parser.py +++ b/metricflow-semantics/metricflow_semantics/query/query_parser.py @@ -11,7 +11,7 @@ ) from dbt_semantic_interfaces.protocols import SavedQuery from dbt_semantic_interfaces.protocols.where_filter import WhereFilter -from dbt_semantic_interfaces.references import SemanticModelReference +from dbt_semantic_interfaces.references import MetricReference, SemanticModelReference from dbt_semantic_interfaces.type_enums import TimeGranularity from metricflow_semantics.assert_one_arg import assert_at_most_one_arg_set @@ -147,13 +147,16 @@ def _get_saved_query(self, saved_query_parameter: SavedQueryParameter) -> SavedQ return matching_saved_queries[0] - @staticmethod - def _metric_time_granularity(time_dimension_specs: Sequence[TimeDimensionSpec]) -> Optional[TimeGranularity]: + def _metric_time_granularity( + self, time_dimension_specs: Sequence[TimeDimensionSpec], queried_metrics: Sequence[MetricReference] + ) -> Optional[TimeGranularity]: matching_specs: Sequence[InstanceSpec] = time_dimension_specs for pattern_to_apply in ( MetricTimePattern(), - DefaultTimeGranularityPattern(), + DefaultTimeGranularityPattern( + metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics + ), NoneDatePartPattern(), ): matching_specs = pattern_to_apply.match(matching_specs) @@ -173,14 +176,19 @@ def _adjust_time_constraint( resolution_dag: GroupByItemResolutionDag, time_dimension_specs_in_query: Sequence[TimeDimensionSpec], time_constraint: TimeRangeConstraint, + metrics_in_query: Sequence[MetricReference], ) -> TimeRangeConstraint: - metric_time_granularity = MetricFlowQueryParser._metric_time_granularity(time_dimension_specs_in_query) + metric_time_granularity = self._metric_time_granularity( + time_dimension_specs=time_dimension_specs_in_query, queried_metrics=metrics_in_query + ) if metric_time_granularity is None: group_by_item_resolver = GroupByItemResolver( manifest_lookup=self._manifest_lookup, resolution_dag=resolution_dag, ) - metric_time_granularity = group_by_item_resolver.resolve_min_metric_time_grain() + metric_time_granularity = group_by_item_resolver.resolve_default_metric_time_grain( + metrics_in_query=metrics_in_query + ) """Change the time range so that the ends are at the ends of the appropriate time granularity windows. @@ -495,6 +503,10 @@ def _parse_and_validate_query( resolution_dag=query_resolution.resolution_dag, time_dimension_specs_in_query=query_spec.time_dimension_specs, time_constraint=time_constraint, + metrics_in_query=tuple( + metric_resolver_input.spec_pattern.metric_reference + for metric_resolver_input in resolver_inputs_for_metrics + ), ) logger.info(f"Time constraint after adjustment is: {time_constraint}") diff --git a/metricflow-semantics/metricflow_semantics/query/query_resolver.py b/metricflow-semantics/metricflow_semantics/query/query_resolver.py index a1517b5462..84a95c25fd 100644 --- a/metricflow-semantics/metricflow_semantics/query/query_resolver.py +++ b/metricflow-semantics/metricflow_semantics/query/query_resolver.py @@ -52,9 +52,8 @@ ResolverInputForQueryLevelWhereFilterIntersection, ResolverInputForWhereFilterIntersection, ) -from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator +from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator, QueryPartForSuggestions from metricflow_semantics.query.validation_rules.query_validator import PostResolutionQueryValidator -from metricflow_semantics.specs.patterns.match_list_pattern import MatchListSpecPattern from metricflow_semantics.specs.query_spec import MetricFlowQuerySpec from metricflow_semantics.specs.spec_classes import ( InstanceSpec, @@ -149,25 +148,26 @@ def _resolve_has_metric_or_group_by_inputs( ) return ResolveMetricOrGroupByItemsResult(input_to_issue_set_mapping=InputToIssueSetMapping.empty_instance()) - @staticmethod def _resolve_group_by_item_input( + self, group_by_item_input: ResolverInputForGroupByItem, group_by_item_resolver: GroupByItemResolver, valid_group_by_item_specs_for_querying: Sequence[LinkableInstanceSpec], + queried_metrics: Sequence[MetricReference], ) -> GroupByItemResolution: suggestion_generator = QueryItemSuggestionGenerator( input_naming_scheme=group_by_item_input.input_obj_naming_scheme, input_str=str(group_by_item_input.input_obj), - candidate_filters=QueryItemSuggestionGenerator.GROUP_BY_ITEM_CANDIDATE_FILTERS - + ( - MatchListSpecPattern( - listed_specs=valid_group_by_item_specs_for_querying, - ), - ), + query_part=QueryPartForSuggestions.GROUP_BY, + metric_lookup=self._manifest_lookup.metric_lookup, + queried_metrics=queried_metrics, + valid_group_by_item_specs_for_querying=valid_group_by_item_specs_for_querying, ) + return group_by_item_resolver.resolve_matching_item_for_querying( spec_pattern=group_by_item_input.spec_pattern, suggestion_generator=suggestion_generator, + queried_metrics=queried_metrics, ) def _resolve_metric_inputs( @@ -190,7 +190,9 @@ def _resolve_metric_inputs( suggestion_generator = QueryItemSuggestionGenerator( input_naming_scheme=MetricNamingScheme(), input_str=str(metric_input.input_obj), - candidate_filters=(), + query_part=QueryPartForSuggestions.METRIC, + metric_lookup=self._manifest_lookup.metric_lookup, + queried_metrics=tuple(metric_input.spec_pattern.metric_reference for metric_input in metric_inputs), ) metric_suggestions = suggestion_generator.input_suggestions(candidate_specs=available_metric_specs) input_to_issue_set_mapping_items.append( @@ -238,10 +240,11 @@ def _resolve_group_by_items_result( group_by_item_specs: List[LinkableInstanceSpec] = [] linkable_element_sets: List[LinkableElementSet] = [] for group_by_item_input in group_by_item_inputs: - resolution = MetricFlowQueryResolver._resolve_group_by_item_input( + resolution = self._resolve_group_by_item_input( group_by_item_resolver=group_by_item_resolver, group_by_item_input=group_by_item_input, valid_group_by_item_specs_for_querying=valid_group_by_item_specs_for_querying, + queried_metrics=metric_references, ) if resolution.issue_set.has_issues: input_to_issue_set_mapping_items.append( diff --git a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py index ba7b59af56..68cc91d4b7 100644 --- a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py +++ b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py @@ -1,19 +1,33 @@ from __future__ import annotations import logging -from typing import Sequence, Tuple +from enum import Enum +from typing import Optional, Sequence, Tuple +from dbt_semantic_interfaces.enum_extension import assert_values_exhausted +from dbt_semantic_interfaces.references import MetricReference + +from metricflow_semantics.model.semantics.metric_lookup import MetricLookup from metricflow_semantics.naming.naming_scheme import QueryItemNamingScheme from metricflow_semantics.query.similarity import top_fuzzy_matches from metricflow_semantics.specs.patterns.base_time_grain import DefaultTimeGranularityPattern +from metricflow_semantics.specs.patterns.match_list_pattern import MatchListSpecPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern -from metricflow_semantics.specs.spec_classes import InstanceSpec +from metricflow_semantics.specs.spec_classes import InstanceSpec, LinkableInstanceSpec logger = logging.getLogger(__name__) +class QueryPartForSuggestions(Enum): + """Indicates which type of query parameter is being suggested.""" + + WHERE_FILTER = "where_filter" + GROUP_BY = "group_by" + METRIC = "metric" + + class QueryItemSuggestionGenerator: """Returns specs that partially match a spec pattern created from user input. Used for suggestions in errors. @@ -22,29 +36,66 @@ class QueryItemSuggestionGenerator: a candidate filter is not needed as any available spec at a resolution node can be used. """ - # Adding these filters so that we don't get multiple suggestions that are similar, but with different - # grains. Some additional thought is needed to tweak this as the base grain may not be the best suggestion. - FILTER_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = (DefaultTimeGranularityPattern(), NoneDatePartPattern()) - GROUP_BY_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = ( - DefaultTimeGranularityPattern(), - NoneDatePartPattern(), - NoGroupByMetricPattern(), - ) - def __init__( # noqa: D107 - self, input_naming_scheme: QueryItemNamingScheme, input_str: str, candidate_filters: Sequence[SpecPattern] + self, + input_naming_scheme: QueryItemNamingScheme, + input_str: str, + query_part: QueryPartForSuggestions, + metric_lookup: MetricLookup, + queried_metrics: Sequence[MetricReference], + valid_group_by_item_specs_for_querying: Optional[Sequence[LinkableInstanceSpec]] = None, ) -> None: self._input_naming_scheme = input_naming_scheme self._input_str = input_str - self._candidate_filters = candidate_filters + self._query_part = query_part + self._metric_lookup = metric_lookup + self._queried_metrics = queried_metrics + self._valid_group_by_item_specs_for_querying = valid_group_by_item_specs_for_querying + + if self._query_part is QueryPartForSuggestions.GROUP_BY and valid_group_by_item_specs_for_querying is None: + raise ValueError( + "QueryItemSuggestionGenerator requires valid_group_by_item_specs_for_querying param when used on group by items." + ) + + @property + def candidate_filters(self) -> Tuple[SpecPattern, ...]: + """Filters to apply before determining suggestions. + + These eensure we don't get multiple suggestions that are similar, but with different grains or date_parts. + """ + default_filters = ( + DefaultTimeGranularityPattern( + metric_lookup=self._metric_lookup, + only_apply_for_metric_time=False, + queried_metrics=self._queried_metrics, + ), + NoneDatePartPattern(), + ) + if self._query_part is QueryPartForSuggestions.WHERE_FILTER: + return default_filters + elif self._query_part is QueryPartForSuggestions.GROUP_BY: + assert self._valid_group_by_item_specs_for_querying, ( + "Group by suggestions require valid_group_by_item_specs_for_querying param." + "This should have been validated on init." + ) + return default_filters + ( + NoGroupByMetricPattern(), + MatchListSpecPattern( + listed_specs=self._valid_group_by_item_specs_for_querying, + ), + ) + elif self._query_part is QueryPartForSuggestions.METRIC: + return () + else: + assert_values_exhausted(self._query_part) def input_suggestions( self, candidate_specs: Sequence[InstanceSpec], max_suggestions: int = 6, ) -> Sequence[str]: - """Return the best specs that match the given pattern from candidate_specs and match the candidate_filer.""" - for candidate_filter in self._candidate_filters: + """Return the best specs that match the given pattern from candidate_specs and match the candidate_filter.""" + for candidate_filter in self.candidate_filters: candidate_specs = candidate_filter.match(candidate_specs) # Use edit distance to figure out the closest matches, so convert the specs to strings. 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 396242ce70..5c30c78cf3 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py @@ -3,9 +3,12 @@ from collections import defaultdict from typing import Dict, List, Sequence, Set +from dbt_semantic_interfaces.references import MetricReference from dbt_semantic_interfaces.type_enums import TimeGranularity from typing_extensions import override +from metricflow_semantics.model.semantics.metric_lookup import MetricLookup +from metricflow_semantics.specs.patterns.metric_time_pattern import MetricTimePattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern from metricflow_semantics.specs.spec_classes import ( InstanceSpec, @@ -20,9 +23,11 @@ class DefaultTimeGranularityPattern(SpecPattern): """A pattern that matches linkable specs, but for time dimension specs, only the one with the default granularity. + # TODO: this might need to change - the below is only relevant for metric_time, right? Update this for all dims. When queried with metrics, default_granularity is specified in the YAML spec for the metrics. If this field is not - set for any of the queried metric(s), defaults to DAY for those metrics unless the minimum granularity is larger than - DAY. In that case, defaults to the smallest available granularity. + set for a metric, it will default to DAY unless the minimum granularity is larger than DAY, in which case it will + default to the minimum granularity. When multiple metrics are queried, the default granularity for the query will + be the largest of the default granularities for the metrics queried. Same defaults apply if no metrics are queried: default to DAY if available for the time dimension queried, else the smallest available granularity. Always defaults to DAY for metric_time if not metrics are queried. @@ -49,8 +54,41 @@ class DefaultTimeGranularityPattern(SpecPattern): time dimension spec with the base grain. """ + def __init__( + self, + metric_lookup: MetricLookup, + only_apply_for_metric_time: bool = False, + queried_metrics: Sequence[MetricReference] = (), + ) -> None: + """Args: + only_apply_for_metric_time: If set, only remove time dimension specs with a non-base grain if it's for + metric_time. This is useful for resolving the default_granularity that metric_time should default to for + a given set of metrics. This is typically set to True when resolving query parameters, and False when + showing suggested group by items for a query (to avoid duplicates of the same time dimension in the list + of suggestions). + queried_metrics: The metrics in cluded in the query. This is used to resolve the default_granularity, which + is set in the metric YAML spec. + + TODO: This is a little odd. This can be replaced once composite patterns are supported. + """ + self._metric_lookup = metric_lookup + self._only_apply_for_metric_time = only_apply_for_metric_time + self._queried_metrics = queried_metrics + @override def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpec]: + if self._only_apply_for_metric_time: + 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( + DefaultTimeGranularityPattern( + metric_lookup=self._metric_lookup, + only_apply_for_metric_time=False, + queried_metrics=self._queried_metrics, + ).match(metric_time_specs) + ) + spec_set = group_specs_by_type(candidate_specs) spec_key_to_grains: Dict[TimeDimensionSpecComparisonKey, Set[TimeGranularity]] = defaultdict(set) @@ -60,12 +98,15 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe spec_key_to_grains[spec_key].add(time_dimension_spec.time_granularity) spec_key_to_specs[spec_key].append(time_dimension_spec) + default_granularity_for_metrics = self._metric_lookup.get_default_granularity_for_metrics(self._queried_metrics) matched_time_dimension_specs: List[TimeDimensionSpec] = [] for spec_key, time_grains in spec_key_to_grains.items(): - # Replace this with new default logic! How does it know for metric time what grain is avail? - # Maybe that logic is done already when passed in here? - # But now we need separate logic if metrics are queried, so we'll need to pass in metrics here (optionally). - matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(min(time_grains))) + grain_to_use = ( + default_granularity_for_metrics + if (default_granularity_for_metrics and spec_key.source_spec.is_metric_time) + else min(time_grains) + ) + matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(grain_to_use)) matching_specs: Sequence[LinkableInstanceSpec] = ( spec_set.dimension_specs diff --git a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_filters.py b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_filters.py index 76b5f61169..c69f5f0de9 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_filters.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_filters.py @@ -9,6 +9,7 @@ from metricflow_semantics.model.semantic_manifest_lookup import SemanticManifestLookup from metricflow_semantics.naming.naming_scheme import QueryItemNamingScheme from metricflow_semantics.naming.object_builder_scheme import ObjectBuilderNamingScheme +from metricflow_semantics.query.group_by_item.filter_spec_resolution.filter_location import WhereFilterLocation from metricflow_semantics.query.group_by_item.group_by_item_resolver import GroupByItemResolver from metricflow_semantics.query.group_by_item.resolution_dag.dag import GroupByItemResolutionDag from metricflow_semantics.test_helpers.config_helpers import MetricFlowTestConfiguration @@ -42,6 +43,7 @@ def test_ambiguous_metric_time_in_query_filter( # noqa: D103 input_str=input_str, spec_pattern=spec_pattern, resolution_node=resolution_dag.sink_node, + filter_location=WhereFilterLocation(metric_references=()), ) assert_object_snapshot_equal( diff --git a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py index 41c0cf6afc..cf8efbd265 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py @@ -41,8 +41,7 @@ def test_ambiguous_metric_time_in_query( # noqa: D103 spec_pattern = ObjectBuilderNamingScheme().spec_pattern(f"TimeDimension('{METRIC_TIME_ELEMENT_NAME}')") result = group_by_item_resolver.resolve_matching_item_for_querying( - spec_pattern=spec_pattern, - suggestion_generator=None, + spec_pattern=spec_pattern, suggestion_generator=None, queried_metrics=resolution_dag.sink_node.metrics_in_query ) if case_id is AmbiguousResolutionQueryId.NO_METRICS: @@ -81,6 +80,7 @@ def test_unavailable_group_by_item_in_derived_metric_parent( result = group_by_item_resolver.resolve_matching_item_for_querying( spec_pattern=spec_pattern, suggestion_generator=None, + queried_metrics=(MetricReference("derived_metric_with_different_parent_time_grains"),), ) assert_object_snapshot_equal( @@ -108,6 +108,7 @@ def test_invalid_group_by_item( # noqa: D103 result = group_by_item_resolver.resolve_matching_item_for_querying( spec_pattern=naming_scheme.spec_pattern(input_str), suggestion_generator=None, + queried_metrics=(MetricReference("monthly_metric_0"), MetricReference("yearly_metric_0")), ) assert_object_snapshot_equal( diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt index 119b0392e5..ac177177ec 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt @@ -31,105 +31,19 @@ FilterSpecResolutionLookUp( ), issue_set=MetricFlowQueryResolutionIssueSet( issues=( - NoCommonItemsInParents( + NoMatchingItemsForMeasure( issue_type=ERROR, query_resolution_path=MetricFlowQueryResolutionPath( resolution_path_nodes=( QueryGroupByItemResolutionNode(node_id=qr_0), MetricGroupByItemResolutionNode(node_id=mtr_2), + MetricGroupByItemResolutionNode(node_id=mtr_1), + MeasureGroupByItemSourceNode(node_id=msr_1), ), ), - parent_candidate_sets=( - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=MONTH, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=MONTH, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_0), - MeasureGroupByItemSourceNode(node_id=msr_0), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_0), - ), - ), - ), - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=YEAR, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=YEAR, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_1), - MeasureGroupByItemSourceNode(node_id=msr_1), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_1), - ), - ), - ), + suggestions=( + "TimeDimension('metric_time', 'month')", + "TimeDimension('yearly_measure_entity__creation_time', 'year')", ), ), ), diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt index 7d9f61426a..142c1737bf 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt @@ -31,110 +31,20 @@ FilterSpecResolutionLookUp( ), issue_set=MetricFlowQueryResolutionIssueSet( issues=( - NoCommonItemsInParents( + NoMatchingItemsForMeasure( issue_type=ERROR, query_resolution_path=MetricFlowQueryResolutionPath( resolution_path_nodes=( QueryGroupByItemResolutionNode(node_id=qr_0), MetricGroupByItemResolutionNode(node_id=mtr_3), MetricGroupByItemResolutionNode(node_id=mtr_2), + MetricGroupByItemResolutionNode(node_id=mtr_1), + MeasureGroupByItemSourceNode(node_id=msr_1), ), ), - parent_candidate_sets=( - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=MONTH, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=MONTH, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_3), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_0), - MeasureGroupByItemSourceNode(node_id=msr_0), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_3), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_0), - ), - ), - ), - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=YEAR, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=YEAR, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_3), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_1), - MeasureGroupByItemSourceNode(node_id=msr_1), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_3), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_1), - ), - ), - ), + suggestions=( + "TimeDimension('metric_time', 'month')", + "TimeDimension('yearly_measure_entity__creation_time', 'year')", ), ), ), diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt index 2fb667f41c..3d94a7ec96 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt @@ -31,105 +31,19 @@ FilterSpecResolutionLookUp( ), issue_set=MetricFlowQueryResolutionIssueSet( issues=( - NoCommonItemsInParents( + NoMatchingItemsForMeasure( issue_type=ERROR, query_resolution_path=MetricFlowQueryResolutionPath( resolution_path_nodes=( QueryGroupByItemResolutionNode(node_id=qr_5), MetricGroupByItemResolutionNode(node_id=mtr_10), + MetricGroupByItemResolutionNode(node_id=mtr_9), + MeasureGroupByItemSourceNode(node_id=msr_8), ), ), - parent_candidate_sets=( - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=MONTH, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=MONTH, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_5), - MetricGroupByItemResolutionNode(node_id=mtr_10), - MetricGroupByItemResolutionNode(node_id=mtr_8), - MeasureGroupByItemSourceNode(node_id=msr_7), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_5), - MetricGroupByItemResolutionNode(node_id=mtr_10), - MetricGroupByItemResolutionNode(node_id=mtr_8), - ), - ), - ), - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=YEAR, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=YEAR, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_5), - MetricGroupByItemResolutionNode(node_id=mtr_10), - MetricGroupByItemResolutionNode(node_id=mtr_9), - MeasureGroupByItemSourceNode(node_id=msr_8), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_5), - MetricGroupByItemResolutionNode(node_id=mtr_10), - MetricGroupByItemResolutionNode(node_id=mtr_9), - ), - ), - ), + suggestions=( + "TimeDimension('metric_time', 'month')", + "TimeDimension('yearly_measure_entity__creation_time', 'year')", ), ), ), diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__metrics_with_different_time_grains__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__metrics_with_different_time_grains__result.txt index 870dd193e7..2dd9caf21d 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__metrics_with_different_time_grains__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__metrics_with_different_time_grains__result.txt @@ -25,111 +25,53 @@ FilterSpecResolutionLookUp( ), ], ), - resolved_linkable_element_set=LinkableElementSet(), - spec_pattern=TimeDimensionPattern( - parameter_set=EntityLinkPatternParameterSet( - fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), - element_name='metric_time', - ), - ), - issue_set=MetricFlowQueryResolutionIssueSet( - issues=( - NoCommonItemsInParents( - issue_type=ERROR, - query_resolution_path=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_3), + resolved_linkable_element_set=LinkableElementSet( + path_key_to_linkable_dimensions={ + ElementPathKey( + element_name='metric_time', + element_type=TIME_DIMENSION, + time_granularity=YEAR, + ): ( + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='monthly_measures_source', ), - ), - parent_candidate_sets=( - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=MONTH, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='monthly_measures_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=MONTH, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_3), - MetricGroupByItemResolutionNode(node_id=mtr_3), - MeasureGroupByItemSourceNode(node_id=msr_3), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_3), - MetricGroupByItemResolutionNode(node_id=mtr_3), - ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='monthly_measures_source', ), ), - GroupByItemCandidateSet( - linkable_element_set=LinkableElementSet( - path_key_to_linkable_dimensions={ - ElementPathKey( - element_name='metric_time', - element_type=TIME_DIMENSION, - time_granularity=YEAR, - ): ( - LinkableDimension( - defined_in_semantic_model=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - element_name='metric_time', - dimension_type=TIME, - join_path=SemanticModelJoinPath( - left_semantic_model_reference=SemanticModelReference( - semantic_model_name='yearly_measure_source', - ), - ), - properties=frozenset( - 'METRIC_TIME', - ), - time_granularity=YEAR, - ), - ), - }, - ), - measure_paths=( - MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_3), - MetricGroupByItemResolutionNode(node_id=mtr_4), - MeasureGroupByItemSourceNode(node_id=msr_4), - ), - ), - ), - path_from_leaf_node=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_3), - MetricGroupByItemResolutionNode(node_id=mtr_4), - ), + properties=frozenset( + 'DERIVED_TIME_GRANULARITY', + 'METRIC_TIME', + ), + time_granularity=YEAR, + ), + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='yearly_measure_source', ), ), + properties=frozenset( + 'METRIC_TIME', + ), + time_granularity=YEAR, ), ), + }, + ), + spec_pattern=TimeDimensionPattern( + parameter_set=EntityLinkPatternParameterSet( + fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), + element_name='metric_time', ), ), filter_location_path=MetricFlowQueryResolutionPath( From d70cd966bc582b1c261d337ab039a2d63d35c0c3 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Tue, 2 Jul 2024 15:57:23 -0700 Subject: [PATCH 09/20] WIP --- .../group_by_item/group_by_item_resolver.py | 10 ++---- .../query/query_parser.py | 2 +- .../query/suggestion_generator.py | 8 ++--- ...e_grain.py => default_time_granularity.py} | 34 +++---------------- 4 files changed, 10 insertions(+), 44 deletions(-) rename metricflow-semantics/metricflow_semantics/specs/patterns/{base_time_grain.py => default_time_granularity.py} (70%) 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 20724cddff..1d97d5ee93 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 DefaultTimeGranularityPattern +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 from metricflow_semantics.specs.patterns.typed_patterns import TimeDimensionPattern @@ -105,9 +105,7 @@ def resolve_matching_item_for_querying( push_down_result = push_down_result.filter_candidates_by_pattern( DefaultTimeGranularityPattern( - metric_lookup=self._manifest_lookup.metric_lookup, - only_apply_for_metric_time=False, - queried_metrics=queried_metrics, + metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics ), ) logger.info( @@ -164,9 +162,7 @@ def resolve_matching_item_for_filters( source_spec_patterns=( spec_pattern, DefaultTimeGranularityPattern( - metric_lookup=self._manifest_lookup.metric_lookup, - only_apply_for_metric_time=False, - queried_metrics=filter_location.metric_references, + metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=filter_location.metric_references ), ), suggestion_generator=suggestion_generator, diff --git a/metricflow-semantics/metricflow_semantics/query/query_parser.py b/metricflow-semantics/metricflow_semantics/query/query_parser.py index 8edfe163e5..34d1ad11b5 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 DefaultTimeGranularityPattern +from metricflow_semantics.specs.patterns.default_time_granularity import DefaultTimeGranularityPattern 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 diff --git a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py index 68cc91d4b7..79f69daafb 100644 --- a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py +++ b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py @@ -10,7 +10,7 @@ from metricflow_semantics.model.semantics.metric_lookup import MetricLookup from metricflow_semantics.naming.naming_scheme import QueryItemNamingScheme from metricflow_semantics.query.similarity import top_fuzzy_matches -from metricflow_semantics.specs.patterns.base_time_grain import DefaultTimeGranularityPattern +from metricflow_semantics.specs.patterns.default_time_granularity import DefaultTimeGranularityPattern from metricflow_semantics.specs.patterns.match_list_pattern import MatchListSpecPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern @@ -64,11 +64,7 @@ def candidate_filters(self) -> Tuple[SpecPattern, ...]: These eensure we don't get multiple suggestions that are similar, but with different grains or date_parts. """ default_filters = ( - DefaultTimeGranularityPattern( - metric_lookup=self._metric_lookup, - only_apply_for_metric_time=False, - queried_metrics=self._queried_metrics, - ), + DefaultTimeGranularityPattern(metric_lookup=self._metric_lookup, queried_metrics=self._queried_metrics), NoneDatePartPattern(), ) if self._query_part is QueryPartForSuggestions.WHERE_FILTER: diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py similarity index 70% rename from metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py rename to metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py index 5c30c78cf3..15a03d4740 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py @@ -8,7 +8,6 @@ from typing_extensions import override from metricflow_semantics.model.semantics.metric_lookup import MetricLookup -from metricflow_semantics.specs.patterns.metric_time_pattern import MetricTimePattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern from metricflow_semantics.specs.spec_classes import ( InstanceSpec, @@ -54,41 +53,16 @@ class DefaultTimeGranularityPattern(SpecPattern): time dimension spec with the base grain. """ - def __init__( - self, - metric_lookup: MetricLookup, - only_apply_for_metric_time: bool = False, - queried_metrics: Sequence[MetricReference] = (), - ) -> None: - """Args: - only_apply_for_metric_time: If set, only remove time dimension specs with a non-base grain if it's for - metric_time. This is useful for resolving the default_granularity that metric_time should default to for - a given set of metrics. This is typically set to True when resolving query parameters, and False when - showing suggested group by items for a query (to avoid duplicates of the same time dimension in the list - of suggestions). - queried_metrics: The metrics in cluded in the query. This is used to resolve the default_granularity, which - is set in the metric YAML spec. - - TODO: This is a little odd. This can be replaced once composite patterns are supported. + def __init__(self, metric_lookup: MetricLookup, queried_metrics: Sequence[MetricReference] = ()) -> None: + """Match only time dimensions with the default granularity for a given query. + + Only affects time dimensions. All other items pass through. """ self._metric_lookup = metric_lookup - self._only_apply_for_metric_time = only_apply_for_metric_time self._queried_metrics = queried_metrics @override def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpec]: - if self._only_apply_for_metric_time: - 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( - DefaultTimeGranularityPattern( - metric_lookup=self._metric_lookup, - only_apply_for_metric_time=False, - queried_metrics=self._queried_metrics, - ).match(metric_time_specs) - ) - spec_set = group_specs_by_type(candidate_specs) spec_key_to_grains: Dict[TimeDimensionSpecComparisonKey, Set[TimeGranularity]] = defaultdict(set) From b7946be6009b0547aefb18e17086f741060ac106 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 3 Jul 2024 09:12:47 -0700 Subject: [PATCH 10/20] WIP --- extra-hatch-configuration/requirements.txt | 2 +- .../requirements.txt | 2 +- .../group_by_item/group_by_item_resolver.py | 1 + .../patterns/default_time_granularity.py | 1 + .../test_matching_item_for_querying.py | 2 +- ...tion_for_invalid_metric_filter__result.txt | 63 ++++++++++++------ ...or_invalid_metric_input_filter__result.txt | 64 +++++++++++++------ ...h_different_parent_time_grains__result.txt | 63 ++++++++++++------ .../DuckDB/test_saved_query__cli_output.txt | 4 ++ ...ery_with_cumulative_metric__cli_output.txt | 2 + ...est_saved_query_with_limit__cli_output.txt | 4 ++ ...est_saved_query_with_where__cli_output.txt | 4 ++ 12 files changed, 148 insertions(+), 64 deletions(-) diff --git a/extra-hatch-configuration/requirements.txt b/extra-hatch-configuration/requirements.txt index ff7374aba3..0a9548db9c 100644 --- a/extra-hatch-configuration/requirements.txt +++ b/extra-hatch-configuration/requirements.txt @@ -1,5 +1,5 @@ Jinja2>=3.1.3 -dbt-semantic-interfaces>=0.6.2 +dbt-semantic-interfaces==0.6.4 more-itertools>=8.10.0, <10.2.0 pydantic>=1.10.0, <3.0 tabulate>=0.8.9 diff --git a/metricflow-semantics/extra-hatch-configuration/requirements.txt b/metricflow-semantics/extra-hatch-configuration/requirements.txt index a69f99c67e..121e58c68d 100644 --- a/metricflow-semantics/extra-hatch-configuration/requirements.txt +++ b/metricflow-semantics/extra-hatch-configuration/requirements.txt @@ -1,5 +1,5 @@ # dbt Cloud depends on metricflow-semantics (dependency set in dbt-mantle), so DSI must always point to a production version here. -dbt-semantic-interfaces>=0.6.2 +dbt-semantic-interfaces>=0.6.4 graphviz>=0.18.2, <0.21 python-dateutil>=2.9.0, <2.10.0 rapidfuzz>=3.0, <4.0 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 1d97d5ee93..d28937f160 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 @@ -127,6 +127,7 @@ def resolve_matching_item_for_querying( ) ), ) + # TODO: return issue if push_down_result.candidate_set.num_candidates < 1 return GroupByItemResolution( spec=push_down_result.candidate_set.specs[0], diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py index 15a03d4740..b4726f0261 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py @@ -73,6 +73,7 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe spec_key_to_specs[spec_key].append(time_dimension_spec) default_granularity_for_metrics = self._metric_lookup.get_default_granularity_for_metrics(self._queried_metrics) + print("default::", self._queried_metrics, default_granularity_for_metrics) matched_time_dimension_specs: List[TimeDimensionSpec] = [] for spec_key, time_grains in spec_key_to_grains.items(): grain_to_use = ( diff --git a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py index cf8efbd265..58109885a7 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py @@ -39,7 +39,7 @@ def test_ambiguous_metric_time_in_query( # noqa: D103 ) spec_pattern = ObjectBuilderNamingScheme().spec_pattern(f"TimeDimension('{METRIC_TIME_ELEMENT_NAME}')") - + print("queried metrics:", resolution_dag.sink_node.metrics_in_query) result = group_by_item_resolver.resolve_matching_item_for_querying( spec_pattern=spec_pattern, suggestion_generator=None, queried_metrics=resolution_dag.sink_node.metrics_in_query ) diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt index ac177177ec..80b32c7364 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt @@ -22,30 +22,53 @@ FilterSpecResolutionLookUp( ), ], ), - resolved_linkable_element_set=LinkableElementSet(), - spec_pattern=TimeDimensionPattern( - parameter_set=EntityLinkPatternParameterSet( - fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), - element_name='metric_time', - ), - ), - issue_set=MetricFlowQueryResolutionIssueSet( - issues=( - NoMatchingItemsForMeasure( - issue_type=ERROR, - query_resolution_path=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_1), - MeasureGroupByItemSourceNode(node_id=msr_1), + resolved_linkable_element_set=LinkableElementSet( + path_key_to_linkable_dimensions={ + ElementPathKey( + element_name='metric_time', + element_type=TIME_DIMENSION, + time_granularity=YEAR, + ): ( + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='monthly_measures_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='monthly_measures_source', + ), + ), + properties=frozenset( + 'DERIVED_TIME_GRANULARITY', + 'METRIC_TIME', ), + time_granularity=YEAR, ), - suggestions=( - "TimeDimension('metric_time', 'month')", - "TimeDimension('yearly_measure_entity__creation_time', 'year')", + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + ), + properties=frozenset( + 'METRIC_TIME', + ), + time_granularity=YEAR, ), ), + }, + ), + spec_pattern=TimeDimensionPattern( + parameter_set=EntityLinkPatternParameterSet( + fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), + element_name='metric_time', ), ), filter_location_path=MetricFlowQueryResolutionPath( diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt index 142c1737bf..139eba5580 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt @@ -22,31 +22,53 @@ FilterSpecResolutionLookUp( ), ], ), - resolved_linkable_element_set=LinkableElementSet(), - spec_pattern=TimeDimensionPattern( - parameter_set=EntityLinkPatternParameterSet( - fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), - element_name='metric_time', - ), - ), - issue_set=MetricFlowQueryResolutionIssueSet( - issues=( - NoMatchingItemsForMeasure( - issue_type=ERROR, - query_resolution_path=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_0), - MetricGroupByItemResolutionNode(node_id=mtr_3), - MetricGroupByItemResolutionNode(node_id=mtr_2), - MetricGroupByItemResolutionNode(node_id=mtr_1), - MeasureGroupByItemSourceNode(node_id=msr_1), + resolved_linkable_element_set=LinkableElementSet( + path_key_to_linkable_dimensions={ + ElementPathKey( + element_name='metric_time', + element_type=TIME_DIMENSION, + time_granularity=YEAR, + ): ( + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='monthly_measures_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='monthly_measures_source', + ), + ), + properties=frozenset( + 'DERIVED_TIME_GRANULARITY', + 'METRIC_TIME', ), + time_granularity=YEAR, ), - suggestions=( - "TimeDimension('metric_time', 'month')", - "TimeDimension('yearly_measure_entity__creation_time', 'year')", + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + ), + properties=frozenset( + 'METRIC_TIME', + ), + time_granularity=YEAR, ), ), + }, + ), + spec_pattern=TimeDimensionPattern( + parameter_set=EntityLinkPatternParameterSet( + fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), + element_name='metric_time', ), ), filter_location_path=MetricFlowQueryResolutionPath( diff --git a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt index 3d94a7ec96..affa1a709f 100644 --- a/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt +++ b/metricflow-semantics/tests_metricflow_semantics/snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt @@ -22,30 +22,53 @@ FilterSpecResolutionLookUp( ), ], ), - resolved_linkable_element_set=LinkableElementSet(), - spec_pattern=TimeDimensionPattern( - parameter_set=EntityLinkPatternParameterSet( - fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), - element_name='metric_time', - ), - ), - issue_set=MetricFlowQueryResolutionIssueSet( - issues=( - NoMatchingItemsForMeasure( - issue_type=ERROR, - query_resolution_path=MetricFlowQueryResolutionPath( - resolution_path_nodes=( - QueryGroupByItemResolutionNode(node_id=qr_5), - MetricGroupByItemResolutionNode(node_id=mtr_10), - MetricGroupByItemResolutionNode(node_id=mtr_9), - MeasureGroupByItemSourceNode(node_id=msr_8), + resolved_linkable_element_set=LinkableElementSet( + path_key_to_linkable_dimensions={ + ElementPathKey( + element_name='metric_time', + element_type=TIME_DIMENSION, + time_granularity=YEAR, + ): ( + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='monthly_measures_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='monthly_measures_source', + ), + ), + properties=frozenset( + 'DERIVED_TIME_GRANULARITY', + 'METRIC_TIME', ), + time_granularity=YEAR, ), - suggestions=( - "TimeDimension('metric_time', 'month')", - "TimeDimension('yearly_measure_entity__creation_time', 'year')", + LinkableDimension( + defined_in_semantic_model=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + element_name='metric_time', + dimension_type=TIME, + join_path=SemanticModelJoinPath( + left_semantic_model_reference=SemanticModelReference( + semantic_model_name='yearly_measure_source', + ), + ), + properties=frozenset( + 'METRIC_TIME', + ), + time_granularity=YEAR, ), ), + }, + ), + spec_pattern=TimeDimensionPattern( + parameter_set=EntityLinkPatternParameterSet( + fields_to_compare=(DATE_PART, ELEMENT_NAME, ENTITY_LINKS), + element_name='metric_time', ), ), filter_location_path=MetricFlowQueryResolutionPath( diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt index 0f30ef0efd..a7c8010ea9 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt @@ -1,3 +1,7 @@ +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY metric_time__day listing__capacity_latest bookings instant_bookings ------------------- -------------------------- ---------- ------------------ 2019-12-01T00:00:00 5 1 0 diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt index 3293139487..0aa1bca736 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt @@ -1,3 +1,5 @@ +default:: (MetricReference(element_name='trailing_2_months_revenue'),) TimeGranularity.DAY +default:: (MetricReference(element_name='trailing_2_months_revenue'),) TimeGranularity.DAY metric_time__day trailing_2_months_revenue ------------------- --------------------------- 2020-01-01T00:00:00 1000 diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt index 01b1c56e28..3a3a7696c8 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt @@ -1,3 +1,7 @@ +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY metric_time__day listing__capacity_latest bookings instant_bookings ------------------- -------------------------- ---------- ------------------ 2019-12-01T00:00:00 5 1 0 diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt index c5cd7d3f3e..e6145049f0 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt @@ -1,3 +1,7 @@ +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY +default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY metric_time__day listing__capacity_latest bookings instant_bookings ------------------- -------------------------- ---------- ------------------ 2019-12-01T00:00:00 5 1 0 From 33209c135a4f20f26386824593fb02b604d30e9a Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 3 Jul 2024 09:49:43 -0700 Subject: [PATCH 11/20] WIP --- .../group_by_item/group_by_item_resolver.py | 7 +- .../specs/patterns/base_time_grain.py | 88 +++++++++++++++++++ .../patterns/default_time_granularity.py | 1 - .../test_cases/itest_granularity.yaml | 2 +- .../integration/test_configured_cases.py | 4 +- .../DuckDB/test_saved_query__cli_output.txt | 4 - ...ery_with_cumulative_metric__cli_output.txt | 2 - ...est_saved_query_with_limit__cli_output.txt | 4 - ...est_saved_query_with_where__cli_output.txt | 4 - 9 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py 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 d28937f160..542cbd67b6 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,6 +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.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,7 +84,7 @@ def resolve_matching_item_for_querying( suggestion_generator: Optional[QueryItemSuggestionGenerator], queried_metrics: Sequence[MetricReference], ) -> GroupByItemResolution: - """Returns the spec that corresponds the one described by spec_pattern and is valid for the query. + """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. @@ -104,9 +105,7 @@ def resolve_matching_item_for_querying( ) push_down_result = push_down_result.filter_candidates_by_pattern( - DefaultTimeGranularityPattern( - metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics - ), + BaseTimeGrainPattern(), ) logger.info( f"Spec pattern:\n" diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py b/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py new file mode 100644 index 0000000000..34b44c1267 --- /dev/null +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py @@ -0,0 +1,88 @@ +from __future__ import annotations + +from collections import defaultdict +from typing import Dict, List, Sequence, Set + +from dbt_semantic_interfaces.type_enums import TimeGranularity +from typing_extensions import override + +from metricflow_semantics.specs.patterns.metric_time_pattern import MetricTimePattern +from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern +from metricflow_semantics.specs.spec_classes import ( + InstanceSpec, + LinkableInstanceSpec, + TimeDimensionSpec, + TimeDimensionSpecComparisonKey, + TimeDimensionSpecField, +) +from metricflow_semantics.specs.spec_set import group_specs_by_type + + +class BaseTimeGrainPattern(SpecPattern): + """A pattern that matches linkable specs, but for time dimension specs, only the one with the finest grain. + + e.g. + + inputs: + [ + TimeDimensionSpec('metric_time', 'day'), + TimeDimensionSpec('metric_time', 'month.'), + DimensionSpec('listing__country'), + ] + + matches: + [ + TimeDimensionSpec('metric_time', 'day'), + DimensionSpec('listing__country'), + ] + + The finest grain represents the defined grain of the time dimension in the semantic model when evaluating specs + of the source. + + This pattern helps to implement matching of group-by-items for where filters - in those cases, an ambiguously + specified group-by-item can only match to time dimension spec with the base grain. + + Also, this is currently used to help implement restrictions on cumulative metrics where they can only be queried + by the base grain of metric_time. + """ + + def __init__(self, only_apply_for_metric_time: bool = False) -> None: + """Initializer. + + Args: + only_apply_for_metric_time: If set, only remove time dimension specs with a non-base grain if it's for + metric_time. This parameter is useful for implementing restrictions on cumulative metrics as they can only + be queried by the base grain of metric_time. + TODO: This is a little odd. This can be replaced once composite patterns are supported. + """ + self._only_apply_for_metric_time = only_apply_for_metric_time + + @override + def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpec]: + if self._only_apply_for_metric_time: + 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)) + + spec_set = group_specs_by_type(candidate_specs) + + spec_key_to_grains: Dict[TimeDimensionSpecComparisonKey, Set[TimeGranularity]] = defaultdict(set) + spec_key_to_specs: Dict[TimeDimensionSpecComparisonKey, List[TimeDimensionSpec]] = defaultdict(list) + for time_dimension_spec in spec_set.time_dimension_specs: + spec_key = time_dimension_spec.comparison_key(exclude_fields=(TimeDimensionSpecField.TIME_GRANULARITY,)) + spec_key_to_grains[spec_key].add(time_dimension_spec.time_granularity) + spec_key_to_specs[spec_key].append(time_dimension_spec) + + matched_time_dimension_specs: List[TimeDimensionSpec] = [] + for spec_key, time_grains in spec_key_to_grains.items(): + matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(min(time_grains))) + + matching_specs: Sequence[LinkableInstanceSpec] = ( + spec_set.dimension_specs + + tuple(matched_time_dimension_specs) + + spec_set.entity_specs + + spec_set.group_by_metric_specs + ) + + return matching_specs diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py index b4726f0261..15a03d4740 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py @@ -73,7 +73,6 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe spec_key_to_specs[spec_key].append(time_dimension_spec) default_granularity_for_metrics = self._metric_lookup.get_default_granularity_for_metrics(self._queried_metrics) - print("default::", self._queried_metrics, default_granularity_for_metrics) matched_time_dimension_specs: List[TimeDimensionSpec] = [] for spec_key, time_grains in spec_key_to_grains.items(): grain_to_use = ( diff --git a/tests_metricflow/integration/test_cases/itest_granularity.yaml b/tests_metricflow/integration/test_cases/itest_granularity.yaml index ea3f974516..ac87e581b2 100644 --- a/tests_metricflow/integration/test_cases/itest_granularity.yaml +++ b/tests_metricflow/integration/test_cases/itest_granularity.yaml @@ -228,7 +228,7 @@ integration_test: time_constraint: ["2020-01-01", "2020-01-02"] check_query: | SELECT - SUM(booking) AS bookings + SUM(1) AS bookings , {{ render_date_trunc("ds", TimeGranularity.MONTH) }} AS metric_time__month FROM {{ source_schema }}.fct_bookings_extended WHERE {{ render_time_constraint("ds", "2020-01-01", "2020-01-31") }} diff --git a/tests_metricflow/integration/test_configured_cases.py b/tests_metricflow/integration/test_configured_cases.py index f381fde20b..4e7f9296a3 100644 --- a/tests_metricflow/integration/test_configured_cases.py +++ b/tests_metricflow/integration/test_configured_cases.py @@ -228,7 +228,8 @@ def filter_not_supported_features( @pytest.mark.parametrize( "name", - CONFIGURED_INTEGRATION_TESTS_REPOSITORY.all_test_case_names, + # CONFIGURED_INTEGRATION_TESTS_REPOSITORY.all_test_case_names, + ["itest_granularity.yaml/daily_metric_with_monthly_time_dimension"], ids=lambda name: f"name={name}", ) def test_case( @@ -319,6 +320,7 @@ def test_case( ) actual = query_result.result_df + assert 0, query_result.sql expected = sql_client.query( jinja2.Template( diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt index a7c8010ea9..0f30ef0efd 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query__cli_output.txt @@ -1,7 +1,3 @@ -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY metric_time__day listing__capacity_latest bookings instant_bookings ------------------- -------------------------- ---------- ------------------ 2019-12-01T00:00:00 5 1 0 diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt index 0aa1bca736..3293139487 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_cumulative_metric__cli_output.txt @@ -1,5 +1,3 @@ -default:: (MetricReference(element_name='trailing_2_months_revenue'),) TimeGranularity.DAY -default:: (MetricReference(element_name='trailing_2_months_revenue'),) TimeGranularity.DAY metric_time__day trailing_2_months_revenue ------------------- --------------------------- 2020-01-01T00:00:00 1000 diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt index 3a3a7696c8..01b1c56e28 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_limit__cli_output.txt @@ -1,7 +1,3 @@ -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY metric_time__day listing__capacity_latest bookings instant_bookings ------------------- -------------------------- ---------- ------------------ 2019-12-01T00:00:00 5 1 0 diff --git a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt index e6145049f0..c5cd7d3f3e 100644 --- a/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt +++ b/tests_metricflow/snapshots/test_cli.py/str/DuckDB/test_saved_query_with_where__cli_output.txt @@ -1,7 +1,3 @@ -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY -default:: (MetricReference(element_name='bookings'), MetricReference(element_name='instant_bookings')) TimeGranularity.DAY metric_time__day listing__capacity_latest bookings instant_bookings ------------------- -------------------------- ---------- ------------------ 2019-12-01T00:00:00 5 1 0 From f30075bb899e4b2cbad33edede9151daa0c3e8bf Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 3 Jul 2024 10:06:44 -0700 Subject: [PATCH 12/20] Restore BaseTimeGrainPattern when adjusting filters --- .../query/query_parser.py | 24 +++++++++---------- .../test_cases/itest_granularity.yaml | 2 +- .../integration/test_configured_cases.py | 4 +--- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/metricflow-semantics/metricflow_semantics/query/query_parser.py b/metricflow-semantics/metricflow_semantics/query/query_parser.py index 34d1ad11b5..7d29286752 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.default_time_granularity import DefaultTimeGranularityPattern +from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern 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 @@ -147,16 +147,14 @@ def _get_saved_query(self, saved_query_parameter: SavedQueryParameter) -> SavedQ return matching_saved_queries[0] - def _metric_time_granularity( - self, time_dimension_specs: Sequence[TimeDimensionSpec], queried_metrics: Sequence[MetricReference] + def _get_smallest_requested_metric_time_granularity( + self, time_dimension_specs: Sequence[TimeDimensionSpec] ) -> Optional[TimeGranularity]: matching_specs: Sequence[InstanceSpec] = time_dimension_specs for pattern_to_apply in ( MetricTimePattern(), - DefaultTimeGranularityPattern( - metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics - ), + BaseTimeGrainPattern(), NoneDatePartPattern(), ): matching_specs = pattern_to_apply.match(matching_specs) @@ -178,10 +176,14 @@ def _adjust_time_constraint( time_constraint: TimeRangeConstraint, metrics_in_query: Sequence[MetricReference], ) -> TimeRangeConstraint: - metric_time_granularity = self._metric_time_granularity( - time_dimension_specs=time_dimension_specs_in_query, queried_metrics=metrics_in_query - ) + """Change the time range so that the ends are at the ends of the requested time granularity windows. + + e.g. [2020-01-15, 2020-2-15] with MONTH granularity -> [2020-01-01, 2020-02-29] + """ + 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. group_by_item_resolver = GroupByItemResolver( manifest_lookup=self._manifest_lookup, resolution_dag=resolution_dag, @@ -190,10 +192,6 @@ def _adjust_time_constraint( metrics_in_query=metrics_in_query ) - """Change the time range so that the ends are at the ends of the appropriate time granularity windows. - - e.g. [2020-01-15, 2020-2-15] with MONTH granularity -> [2020-01-01, 2020-02-29] - """ return self._time_period_adjuster.expand_time_constraint_to_fill_granularity( time_constraint=time_constraint, granularity=metric_time_granularity, diff --git a/tests_metricflow/integration/test_cases/itest_granularity.yaml b/tests_metricflow/integration/test_cases/itest_granularity.yaml index ac87e581b2..b47d11ec58 100644 --- a/tests_metricflow/integration/test_cases/itest_granularity.yaml +++ b/tests_metricflow/integration/test_cases/itest_granularity.yaml @@ -221,7 +221,7 @@ integration_test: --- integration_test: name: daily_metric_with_monthly_time_dimension - description: Query a metric with a month-granularity time dimensions. + description: Query a metric with a month-granularity time dimensions. Filter should expand to requested granularity. model: EXTENDED_DATE_MODEL metrics: ["bookings"] group_bys: ["metric_time__month"] diff --git a/tests_metricflow/integration/test_configured_cases.py b/tests_metricflow/integration/test_configured_cases.py index 4e7f9296a3..f381fde20b 100644 --- a/tests_metricflow/integration/test_configured_cases.py +++ b/tests_metricflow/integration/test_configured_cases.py @@ -228,8 +228,7 @@ def filter_not_supported_features( @pytest.mark.parametrize( "name", - # CONFIGURED_INTEGRATION_TESTS_REPOSITORY.all_test_case_names, - ["itest_granularity.yaml/daily_metric_with_monthly_time_dimension"], + CONFIGURED_INTEGRATION_TESTS_REPOSITORY.all_test_case_names, ids=lambda name: f"name={name}", ) def test_case( @@ -320,7 +319,6 @@ def test_case( ) actual = query_result.result_df - assert 0, query_result.sql expected = sql_client.query( jinja2.Template( From 56e3990728c492cade1e1f0f2ddb7ec03eb0b062 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 3 Jul 2024 11:05:14 -0700 Subject: [PATCH 13/20] Cleanup --- tests_metricflow/integration/test_cases/itest_granularity.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests_metricflow/integration/test_cases/itest_granularity.yaml b/tests_metricflow/integration/test_cases/itest_granularity.yaml index b47d11ec58..34e3cec01d 100644 --- a/tests_metricflow/integration/test_cases/itest_granularity.yaml +++ b/tests_metricflow/integration/test_cases/itest_granularity.yaml @@ -228,7 +228,7 @@ integration_test: time_constraint: ["2020-01-01", "2020-01-02"] check_query: | SELECT - SUM(1) AS bookings + SUM(booking) AS bookings , {{ render_date_trunc("ds", TimeGranularity.MONTH) }} AS metric_time__month FROM {{ source_schema }}.fct_bookings_extended WHERE {{ render_time_constraint("ds", "2020-01-01", "2020-01-31") }} From 0c1d3ab088ee20d010f149ed38a67e64e494357e Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Mon, 8 Jul 2024 17:00:17 -0700 Subject: [PATCH 14/20] WIP --- .changes/unreleased/Features-20240628-074617.yaml | 2 +- .../query/group_by_item/group_by_item_resolver.py | 10 +++++++++- .../simple_manifest/metrics.yaml | 6 ++++++ .../semantic_models/listings_latest.yaml | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.changes/unreleased/Features-20240628-074617.yaml b/.changes/unreleased/Features-20240628-074617.yaml index 2ad17eaf7d..daecb746f9 100644 --- a/.changes/unreleased/Features-20240628-074617.yaml +++ b/.changes/unreleased/Features-20240628-074617.yaml @@ -1,5 +1,5 @@ kind: Features -body: Use default_grain to resolve metric_time. +body: Use default_granularity to resolve metric_time. time: 2024-06-28T07:46:17.768805-07:00 custom: Author: courtneyholcomb 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 542cbd67b6..beeb358727 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 @@ -104,8 +104,16 @@ def resolve_matching_item_for_querying( issue_set=push_down_result.issue_set, ) + # TODO: this isn't working, figure out why + filter_to_use = ( + DefaultTimeGranularityPattern( + metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics + ) + if queried_metrics + else + ) push_down_result = push_down_result.filter_candidates_by_pattern( - BaseTimeGrainPattern(), + filter_to_use, ) logger.info( f"Spec pattern:\n" 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 877453249f..0923e2b4af 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 @@ -92,6 +92,7 @@ metric: type: simple type_params: measure: listings + default_granularity: day --- metric: name: "lux_listings" @@ -100,6 +101,7 @@ metric: type_params: measure: listings filter: "{{ Dimension('listing__is_lux_latest') }}" + default_granularity: day --- metric: name: "smallest_listing" @@ -107,6 +109,7 @@ metric: type: simple type_params: measure: smallest_listing + default_granularity: day --- metric: name: "largest_listing" @@ -114,6 +117,7 @@ metric: type: simple type_params: measure: largest_listing + default_granularity: day --- metric: name: "identity_verifications" @@ -769,6 +773,7 @@ metric: type_params: measure: listings filter: "{{ Metric('bookings', ['listing']) }} > 2" + default_granularity: day --- metric: name: popular_listing_bookings_per_booker @@ -780,3 +785,4 @@ metric: denominator: name: listings filter: "{{ Metric('views', ['listing']) }} > 10" + default_granularity: day diff --git a/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml b/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml index c14800050e..7de7c8bf30 100644 --- a/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml +++ b/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml @@ -26,7 +26,7 @@ semantic_model: type: time expr: created_at type_params: - time_granularity: day + time_granularity: second - name: created_at type: time type_params: From 54d19f68a049f891ef07299ec7082c03eb597adf Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Tue, 9 Jul 2024 14:43:52 -0700 Subject: [PATCH 15/20] WIP --- .../group_by_item/group_by_item_resolver.py | 12 +++---- .../simple_manifest/metrics.yaml | 8 ++--- .../semantic_models/listings_latest.yaml | 2 +- .../test_matching_item_for_querying.py | 5 ++- .../test_granularity_date_part_rendering.py | 33 +++++++++++++++++++ 5 files changed, 44 insertions(+), 16 deletions(-) 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 beeb358727..941d202f27 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 @@ -105,15 +105,11 @@ def resolve_matching_item_for_querying( ) # TODO: this isn't working, figure out why - filter_to_use = ( - DefaultTimeGranularityPattern( - metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics - ) - if queried_metrics - else - ) + # filter_to_use = 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, + BaseTimeGrainPattern(), ) logger.info( f"Spec pattern:\n" 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 0923e2b4af..e56d69c8bb 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 @@ -92,7 +92,6 @@ metric: type: simple type_params: measure: listings - default_granularity: day --- metric: name: "lux_listings" @@ -101,7 +100,6 @@ metric: type_params: measure: listings filter: "{{ Dimension('listing__is_lux_latest') }}" - default_granularity: day --- metric: name: "smallest_listing" @@ -109,7 +107,6 @@ metric: type: simple type_params: measure: smallest_listing - default_granularity: day --- metric: name: "largest_listing" @@ -117,7 +114,7 @@ metric: type: simple type_params: measure: largest_listing - default_granularity: day + default_granularity: month --- metric: name: "identity_verifications" @@ -773,7 +770,6 @@ metric: type_params: measure: listings filter: "{{ Metric('bookings', ['listing']) }} > 2" - default_granularity: day --- metric: name: popular_listing_bookings_per_booker @@ -785,4 +781,4 @@ metric: denominator: name: listings filter: "{{ Metric('views', ['listing']) }} > 10" - default_granularity: day + default_granularity: week \ No newline at end of file diff --git a/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml b/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml index 7de7c8bf30..c14800050e 100644 --- a/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml +++ b/metricflow-semantics/metricflow_semantics/test_helpers/semantic_manifest_yamls/simple_manifest/semantic_models/listings_latest.yaml @@ -26,7 +26,7 @@ semantic_model: type: time expr: created_at type_params: - time_granularity: second + time_granularity: day - name: created_at type: time type_params: diff --git a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py index 58109885a7..cc203505cf 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/group_by_item/test_matching_item_for_querying.py @@ -16,6 +16,9 @@ from metricflow_semantics.query.group_by_item.resolution_dag.resolution_nodes.metric_resolution_node import ( MetricGroupByItemResolutionNode, ) +from metricflow_semantics.query.group_by_item.resolution_dag.resolution_nodes.query_resolution_node import ( + QueryGroupByItemResolutionNode, +) from metricflow_semantics.test_helpers.config_helpers import MetricFlowTestConfiguration from metricflow_semantics.test_helpers.metric_time_dimension import MTD_SPEC_DAY, MTD_SPEC_MONTH, MTD_SPEC_YEAR from metricflow_semantics.test_helpers.snapshot_helpers import assert_object_snapshot_equal @@ -39,7 +42,7 @@ def test_ambiguous_metric_time_in_query( # noqa: D103 ) spec_pattern = ObjectBuilderNamingScheme().spec_pattern(f"TimeDimension('{METRIC_TIME_ELEMENT_NAME}')") - print("queried metrics:", resolution_dag.sink_node.metrics_in_query) + assert isinstance(resolution_dag.sink_node, QueryGroupByItemResolutionNode) result = group_by_item_resolver.resolve_matching_item_for_querying( spec_pattern=spec_pattern, suggestion_generator=None, queried_metrics=resolution_dag.sink_node.metrics_in_query ) diff --git a/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py b/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py index eeebee90d2..6b1b6aaf00 100644 --- a/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py +++ b/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py @@ -101,3 +101,36 @@ def test_offset_window_with_date_part( # noqa: D103 dataflow_plan_builder=dataflow_plan_builder, query_spec=query_spec, ) + + +@pytest.mark.sql_engine_snapshot +def test_offset_window_with_date_part( # noqa: D103 + request: FixtureRequest, + mf_test_configuration: MetricFlowTestConfiguration, + dataflow_plan_builder: DataflowPlanBuilder, + dataflow_to_sql_converter: DataflowToSqlQueryPlanConverter, + sql_client: SqlClient, +) -> None: + query_spec = MetricFlowQuerySpec( + metric_specs=(MetricSpec(element_name="bookings_growth_2_weeks"),), + time_dimension_specs=( + DataSet.metric_time_dimension_spec(time_granularity=TimeGranularity.DAY, date_part=DatePart.DOW), + ), + ) + + render_and_check( + request=request, + mf_test_configuration=mf_test_configuration, + dataflow_to_sql_converter=dataflow_to_sql_converter, + sql_client=sql_client, + dataflow_plan_builder=dataflow_plan_builder, + query_spec=query_spec, + ) + + +# Tests to write: +# Simple metric with default granularity +# Derived metric with default granularity +# Defaults to agg_time_dimension +# Default when day isn't avail +# Default for derived metric with diff agg time grains From 906fd0a819fd97955badb668bdbfff02ccd79191 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Tue, 9 Jul 2024 16:06:21 -0700 Subject: [PATCH 16/20] Write query parser tests --- .../simple_manifest/metrics.yaml | 2 +- .../query/test_query_parser.py | 61 ++++++++++++++++++- .../test_granularity_date_part_rendering.py | 33 ---------- 3 files changed, 61 insertions(+), 35 deletions(-) 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 e56d69c8bb..ce1854fd06 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 @@ -781,4 +781,4 @@ metric: denominator: name: listings filter: "{{ Metric('views', ['listing']) }} > 10" - default_granularity: week \ No newline at end of file + default_granularity: week diff --git a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py index 79f1e57e34..3888fa1c9f 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py @@ -30,7 +30,7 @@ from metricflow_semantics.test_helpers.example_project_configuration import ( EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE, ) -from metricflow_semantics.test_helpers.metric_time_dimension import MTD +from metricflow_semantics.test_helpers.metric_time_dimension import MTD, MTD_SPEC_DAY, MTD_SPEC_MONTH, MTD_SPEC_YEAR from metricflow_semantics.test_helpers.snapshot_helpers import assert_object_snapshot_equal logger = logging.getLogger(__name__) @@ -53,6 +53,10 @@ expr: "1" agg: sum create_metric: true + - name: monthly_bookings + expr: "1" + agg: sum + agg_time_dimension: ds_month dimensions: - name: is_instant @@ -61,6 +65,10 @@ type: time type_params: time_granularity: day + - name: ds_month + type: time + type_params: + time_granularity: month primary_entity: booking @@ -68,6 +76,33 @@ - name: listing type: foreign expr: listing_id + + --- + metric: + name: instant_bookings + description: instant bookings + type: simple + type_params: + measure: bookings + filter: "{{ Dimension('booking__is_instant') }}" + default_granularity: year + --- + metric: + name: month_bookings + description: monthly bookings + type: simple + type_params: + measure: monthly_bookings + --- + metric: + name: instant_plus_months_bookings + description: instant plus month bookings + type: derived + type_params: + expr: instant_bookings + month_bookings + metrics: + - name: instant_bookings + - name: month_bookings """ ) @@ -626,3 +661,27 @@ def test_invalid_group_by_metric(bookings_query_parser: MetricFlowQueryParser) - bookings_query_parser.parse_and_validate_query( metric_names=("bookings",), where_constraint_str="{{ Metric('listings', ['garbage']) }} > 1" ) + + +def test_default_granularity(bookings_query_parser: MetricFlowQueryParser) -> None: + """Tests different scenarios using default granularity.""" + # Metric with default_granularity set + query_spec = bookings_query_parser.parse_and_validate_query( + metric_names=("instant_bookings",), group_by_names=("metric_time",) + ).query_spec + assert len(query_spec.time_dimension_specs) == 1 + assert query_spec.time_dimension_specs[0] == MTD_SPEC_YEAR + + # Metric without default_granularity set + query_spec = bookings_query_parser.parse_and_validate_query( + metric_names=("month_bookings",), group_by_names=("metric_time",) + ).query_spec + assert len(query_spec.time_dimension_specs) == 1 + assert query_spec.time_dimension_specs[0] == MTD_SPEC_MONTH + + # Derived metric with different agg_time_dimensions and no default granularity set + query_spec = bookings_query_parser.parse_and_validate_query( + metric_names=("instant_plus_months_bookings",), group_by_names=("metric_time",) + ).query_spec + assert len(query_spec.time_dimension_specs) == 1 + assert query_spec.time_dimension_specs[0] == MTD_SPEC_MONTH diff --git a/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py b/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py index 6b1b6aaf00..eeebee90d2 100644 --- a/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py +++ b/tests_metricflow/query_rendering/test_granularity_date_part_rendering.py @@ -101,36 +101,3 @@ def test_offset_window_with_date_part( # noqa: D103 dataflow_plan_builder=dataflow_plan_builder, query_spec=query_spec, ) - - -@pytest.mark.sql_engine_snapshot -def test_offset_window_with_date_part( # noqa: D103 - request: FixtureRequest, - mf_test_configuration: MetricFlowTestConfiguration, - dataflow_plan_builder: DataflowPlanBuilder, - dataflow_to_sql_converter: DataflowToSqlQueryPlanConverter, - sql_client: SqlClient, -) -> None: - query_spec = MetricFlowQuerySpec( - metric_specs=(MetricSpec(element_name="bookings_growth_2_weeks"),), - time_dimension_specs=( - DataSet.metric_time_dimension_spec(time_granularity=TimeGranularity.DAY, date_part=DatePart.DOW), - ), - ) - - render_and_check( - request=request, - mf_test_configuration=mf_test_configuration, - dataflow_to_sql_converter=dataflow_to_sql_converter, - sql_client=sql_client, - dataflow_plan_builder=dataflow_plan_builder, - query_spec=query_spec, - ) - - -# Tests to write: -# Simple metric with default granularity -# Derived metric with default granularity -# Defaults to agg_time_dimension -# Default when day isn't avail -# Default for derived metric with diff agg time grains From adb5274c269d99c52795a2d73bc5ffa77cb0704f Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 10 Jul 2024 10:08:51 -0700 Subject: [PATCH 17/20] Debug & get working! --- .../query/group_by_item/group_by_item_resolver.py | 12 +++++------- .../specs/patterns/default_time_granularity.py | 9 ++++++++- .../query/test_query_parser.py | 7 +++++++ .../test_cases/itest_measure_aggregations.yaml | 2 +- 4 files changed, 21 insertions(+), 9 deletions(-) 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 941d202f27..32c48b9ae5 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 @@ -87,7 +87,7 @@ def resolve_matching_item_for_querying( """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. + common grain is returned, unless the spec is metric_time, in which case the default grain is returned. """ push_down_visitor = _PushDownGroupByItemCandidatesVisitor( manifest_lookup=self._manifest_lookup, @@ -104,13 +104,12 @@ def resolve_matching_item_for_querying( issue_set=push_down_result.issue_set, ) - # TODO: this isn't working, figure out why - # filter_to_use = DefaultTimeGranularityPattern( - # metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics - # ) push_down_result = push_down_result.filter_candidates_by_pattern( - BaseTimeGrainPattern(), + DefaultTimeGranularityPattern( + metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics + ) ) + logger.info( f"Spec pattern:\n" f"{indent(mf_pformat(spec_pattern))}\n" @@ -130,7 +129,6 @@ def resolve_matching_item_for_querying( ) ), ) - # TODO: return issue if push_down_result.candidate_set.num_candidates < 1 return GroupByItemResolution( spec=push_down_result.candidate_set.specs[0], diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py index 15a03d4740..de53cb9ff2 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py @@ -77,7 +77,14 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe for spec_key, time_grains in spec_key_to_grains.items(): grain_to_use = ( default_granularity_for_metrics - if (default_granularity_for_metrics and spec_key.source_spec.is_metric_time) + if ( + default_granularity_for_metrics + # Use default_granularity for metric_time, but minimum granularity for all other time dims. + and spec_key.source_spec.is_metric_time + # If default_granularity is not in the available options, then time granularity was probably + # specified in the request and does not need a default here. + and default_granularity_for_metrics in time_grains + ) else min(time_grains) ) matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(grain_to_use)) diff --git a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py index 3888fa1c9f..5adf2ec399 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py @@ -685,3 +685,10 @@ def test_default_granularity(bookings_query_parser: MetricFlowQueryParser) -> No ).query_spec assert len(query_spec.time_dimension_specs) == 1 assert query_spec.time_dimension_specs[0] == MTD_SPEC_MONTH + + # Using non-metric_time - should ignore default granularity + query_spec = bookings_query_parser.parse_and_validate_query( + metric_names=("instant_bookings",), group_by_names=("booking__ds",) + ).query_spec + assert len(query_spec.time_dimension_specs) == 1 + assert query_spec.time_dimension_specs[0].time_granularity == TimeGranularity.DAY diff --git a/tests_metricflow/integration/test_cases/itest_measure_aggregations.yaml b/tests_metricflow/integration/test_cases/itest_measure_aggregations.yaml index cc98ed9828..e3fc977427 100644 --- a/tests_metricflow/integration/test_cases/itest_measure_aggregations.yaml +++ b/tests_metricflow/integration/test_cases/itest_measure_aggregations.yaml @@ -8,7 +8,7 @@ integration_test: check_query: | SELECT MAX(capacity) as largest_listing - , created_at AS metric_time__day + , {{ render_date_trunc("created_at", TimeGranularity.MONTH) }} AS metric_time__month FROM {{ source_schema }}.dim_listings_latest GROUP BY 2 From ab3e5273d3c0c524e617c0a4e6c0d00576b3f761 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 10 Jul 2024 10:38:08 -0700 Subject: [PATCH 18/20] Fix time constraint adjustment --- .../model/semantics/linkable_element_set.py | 2 +- .../query/group_by_item/group_by_item_resolver.py | 15 +++++++++++---- .../metricflow_semantics/query/query_parser.py | 15 +++++---------- .../specs/patterns/base_time_grain.py | 6 ++++-- 4 files changed, 21 insertions(+), 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 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) From 6cf0243caa7f7804c6de54245c2b0acaa7b8f4ee Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 10 Jul 2024 10:42:02 -0700 Subject: [PATCH 19/20] Tests to make sure requested grain is still respected --- .../semantic_manifest_yamls/simple_manifest/metrics.yaml | 1 + .../integration/query_output/test_fill_nulls_with_0.py | 8 ++++---- .../integration/test_cases/itest_metrics.yaml | 4 ++-- .../query_rendering/test_fill_nulls_with_rendering.py | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) 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..94111e2df3 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), ) @@ -103,8 +103,8 @@ def test_fill_nulls_with_0_multi_metric_query( # noqa: D103 query_result = it_helpers.mf_engine.query( MetricFlowQueryRequest.create_with_random_request_id( metric_names=["bookings_fill_nulls_with_0", "views"], - 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..a7967b7e8f 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", "grain": "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", "grain": "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), From c502339f4d01d1551a0b4d11987f7efca7cc72c1 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Wed, 10 Jul 2024 11:23:46 -0700 Subject: [PATCH 20/20] Yusssss --- .../group_by_item/group_by_item_resolver.py | 38 +++++++------ .../query/query_parser.py | 4 +- .../query/suggestion_generator.py | 12 +++- ....py => metric_time_default_granularity.py} | 57 ++++++++----------- .../{base_time_grain.py => min_time_grain.py} | 0 .../specs/query_param_implementations.py | 6 -- .../query/test_query_parser.py | 4 +- 7 files changed, 60 insertions(+), 61 deletions(-) rename metricflow-semantics/metricflow_semantics/specs/patterns/{default_time_granularity.py => metric_time_default_granularity.py} (51%) rename metricflow-semantics/metricflow_semantics/specs/patterns/{base_time_grain.py => min_time_grain.py} (100%) 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 40afd31a78..2c9274ee69 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,8 +28,8 @@ MetricFlowQueryResolutionIssueSet, ) from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator, QueryPartForSuggestions -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.metric_time_default_granularity import MetricTimeDefaultGranularityPattern +from metricflow_semantics.specs.patterns.min_time_grain import MinimumTimeGrainPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern from metricflow_semantics.specs.patterns.typed_patterns import TimeDimensionPattern @@ -83,14 +83,14 @@ def resolve_matching_item_for_querying( spec_pattern: SpecPattern, suggestion_generator: Optional[QueryItemSuggestionGenerator], queried_metrics: Sequence[MetricReference], - use_minimum_grain: bool = False, + only_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. + If only_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, @@ -107,14 +107,17 @@ def resolve_matching_item_for_querying( issue_set=push_down_result.issue_set, ) - 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) + filters_to_use: Tuple[SpecPattern, ...] = (MinimumTimeGrainPattern(),) + if not only_use_minimum_grain: + # Default pattern must come first to avoid removing default grain options prematurely. + filters_to_use = ( + MetricTimeDefaultGranularityPattern( + metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=queried_metrics + ), + ) + filters_to_use + + for filter_to_use in filters_to_use: + push_down_result = push_down_result.filter_candidates_by_pattern(filter_to_use) logger.info( f"Spec pattern:\n" @@ -169,9 +172,12 @@ def resolve_matching_item_for_filters( manifest_lookup=self._manifest_lookup, source_spec_patterns=( spec_pattern, - DefaultTimeGranularityPattern( + # MetricTimeDefaultGranularityPattern must come before MinimumTimeGrainPattern to ensure we don't remove the + # default grain from candiate set prematurely. + MetricTimeDefaultGranularityPattern( metric_lookup=self._manifest_lookup.metric_lookup, queried_metrics=filter_location.metric_references ), + MinimumTimeGrainPattern(), ), suggestion_generator=suggestion_generator, ) @@ -230,8 +236,8 @@ def resolve_available_items( issue_set=push_down_result.issue_set, ) - def resolve_default_metric_time_grain(self, metrics_in_query: Sequence[MetricReference]) -> TimeGranularity: - """Returns the default time grain of metric_time for querying.""" + def resolve_min_metric_time_grain(self, metrics_in_query: Sequence[MetricReference]) -> TimeGranularity: + """Returns the finest time grain of metric_time for querying.""" metric_time_grain_resolution = self.resolve_matching_item_for_querying( spec_pattern=TimeDimensionPattern.from_call_parameter_set( TimeDimensionCallParameterSet( @@ -241,7 +247,7 @@ def resolve_default_metric_time_grain(self, metrics_in_query: Sequence[MetricRef ), suggestion_generator=None, queried_metrics=metrics_in_query, - use_minimum_grain=True, + only_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 56078e32e0..0b52bdd734 100644 --- a/metricflow-semantics/metricflow_semantics/query/query_parser.py +++ b/metricflow-semantics/metricflow_semantics/query/query_parser.py @@ -52,8 +52,8 @@ ResolverInputForQuery, ResolverInputForQueryLevelWhereFilterIntersection, ) -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.min_time_grain import MinimumTimeGrainPattern from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern from metricflow_semantics.specs.query_param_implementations import DimensionOrEntityParameter, MetricParameter from metricflow_semantics.specs.query_spec import MetricFlowQuerySpec @@ -187,7 +187,7 @@ def _adjust_time_constraint( manifest_lookup=self._manifest_lookup, resolution_dag=resolution_dag, ) - metric_time_granularity = group_by_item_resolver.resolve_default_metric_time_grain( + metric_time_granularity = group_by_item_resolver.resolve_min_metric_time_grain( metrics_in_query=metrics_in_query ) diff --git a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py index 79f69daafb..06b0a4a18b 100644 --- a/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py +++ b/metricflow-semantics/metricflow_semantics/query/suggestion_generator.py @@ -10,8 +10,9 @@ from metricflow_semantics.model.semantics.metric_lookup import MetricLookup from metricflow_semantics.naming.naming_scheme import QueryItemNamingScheme from metricflow_semantics.query.similarity import top_fuzzy_matches -from metricflow_semantics.specs.patterns.default_time_granularity import DefaultTimeGranularityPattern from metricflow_semantics.specs.patterns.match_list_pattern import MatchListSpecPattern +from metricflow_semantics.specs.patterns.metric_time_default_granularity import MetricTimeDefaultGranularityPattern +from metricflow_semantics.specs.patterns.min_time_grain import MinimumTimeGrainPattern from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern @@ -61,11 +62,16 @@ def __init__( # noqa: D107 def candidate_filters(self) -> Tuple[SpecPattern, ...]: """Filters to apply before determining suggestions. - These eensure we don't get multiple suggestions that are similar, but with different grains or date_parts. + These ensure we don't get multiple suggestions that are similar, but with different grains or date_parts. """ default_filters = ( - DefaultTimeGranularityPattern(metric_lookup=self._metric_lookup, queried_metrics=self._queried_metrics), NoneDatePartPattern(), + # MetricTimeDefaultGranularityPattern must come before MinimumTimeGrainPattern to ensure we don't remove the + # default grain from candiate set prematurely. + MetricTimeDefaultGranularityPattern( + metric_lookup=self._metric_lookup, queried_metrics=self._queried_metrics + ), + MinimumTimeGrainPattern(), ) if self._query_part is QueryPartForSuggestions.WHERE_FILTER: return default_filters diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py b/metricflow-semantics/metricflow_semantics/specs/patterns/metric_time_default_granularity.py similarity index 51% rename from metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py rename to metricflow-semantics/metricflow_semantics/specs/patterns/metric_time_default_granularity.py index de53cb9ff2..2c61f407e4 100644 --- a/metricflow-semantics/metricflow_semantics/specs/patterns/default_time_granularity.py +++ b/metricflow-semantics/metricflow_semantics/specs/patterns/metric_time_default_granularity.py @@ -19,30 +19,24 @@ from metricflow_semantics.specs.spec_set import group_specs_by_type -class DefaultTimeGranularityPattern(SpecPattern): - """A pattern that matches linkable specs, but for time dimension specs, only the one with the default granularity. +class MetricTimeDefaultGranularityPattern(SpecPattern): + """A pattern that matches metric_time specs, applying the default granularity for the requested metrics. - # TODO: this might need to change - the below is only relevant for metric_time, right? Update this for all dims. - When queried with metrics, default_granularity is specified in the YAML spec for the metrics. If this field is not - set for a metric, it will default to DAY unless the minimum granularity is larger than DAY, in which case it will - default to the minimum granularity. When multiple metrics are queried, the default granularity for the query will - be the largest of the default granularities for the metrics queried. + If granularity is already selected or if no metrics were queried, no default is needed. All other specs are passed through. - Same defaults apply if no metrics are queried: default to DAY if available for the time dimension queried, else the - smallest available granularity. Always defaults to DAY for metric_time if not metrics are queried. - - e.g. + e.g., if a metric with default_granularity MONTH is queried inputs: [ TimeDimensionSpec('metric_time', 'day'), - TimeDimensionSpec('metric_time', 'month.'), + TimeDimensionSpec('metric_time', 'week'), + TimeDimensionSpec('metric_time', 'month'), DimensionSpec('listing__country'), ] matches: [ - TimeDimensionSpec('metric_time', 'day'), + TimeDimensionSpec('metric_time', 'month'), DimensionSpec('listing__country'), ] @@ -63,35 +57,34 @@ def __init__(self, metric_lookup: MetricLookup, queried_metrics: Sequence[Metric @override def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpec]: + default_granularity_for_metrics = self._metric_lookup.get_default_granularity_for_metrics(self._queried_metrics) spec_set = group_specs_by_type(candidate_specs) + # If there are no metrics or metric_time specs in the query, skip this filter. + if not (default_granularity_for_metrics and spec_set.metric_time_specs): + return candidate_specs spec_key_to_grains: Dict[TimeDimensionSpecComparisonKey, Set[TimeGranularity]] = defaultdict(set) spec_key_to_specs: Dict[TimeDimensionSpecComparisonKey, List[TimeDimensionSpec]] = defaultdict(list) - for time_dimension_spec in spec_set.time_dimension_specs: - spec_key = time_dimension_spec.comparison_key(exclude_fields=(TimeDimensionSpecField.TIME_GRANULARITY,)) - spec_key_to_grains[spec_key].add(time_dimension_spec.time_granularity) - spec_key_to_specs[spec_key].append(time_dimension_spec) + for metric_time_spec in spec_set.metric_time_specs: + spec_key = metric_time_spec.comparison_key(exclude_fields=(TimeDimensionSpecField.TIME_GRANULARITY,)) + spec_key_to_grains[spec_key].add(metric_time_spec.time_granularity) + spec_key_to_specs[spec_key].append(metric_time_spec) - default_granularity_for_metrics = self._metric_lookup.get_default_granularity_for_metrics(self._queried_metrics) - matched_time_dimension_specs: List[TimeDimensionSpec] = [] + matched_metric_time_specs: List[TimeDimensionSpec] = [] for spec_key, time_grains in spec_key_to_grains.items(): - grain_to_use = ( - default_granularity_for_metrics - if ( - default_granularity_for_metrics - # Use default_granularity for metric_time, but minimum granularity for all other time dims. - and spec_key.source_spec.is_metric_time - # If default_granularity is not in the available options, then time granularity was probably - # specified in the request and does not need a default here. - and default_granularity_for_metrics in time_grains + if default_granularity_for_metrics in time_grains: + matched_metric_time_specs.append( + spec_key_to_specs[spec_key][0].with_grain(default_granularity_for_metrics) ) - else min(time_grains) - ) - matched_time_dimension_specs.append(spec_key_to_specs[spec_key][0].with_grain(grain_to_use)) + else: + # If default_granularity is not in the available options, then time granularity was specified in the request + # and a default is not needed here. Pass all options through. + matched_metric_time_specs.extend(spec_key_to_specs[spec_key]) matching_specs: Sequence[LinkableInstanceSpec] = ( spec_set.dimension_specs - + tuple(matched_time_dimension_specs) + + tuple(matched_metric_time_specs) + + tuple(spec for spec in spec_set.time_dimension_specs if not spec.is_metric_time) + spec_set.entity_specs + spec_set.group_by_metric_specs ) diff --git a/metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py b/metricflow-semantics/metricflow_semantics/specs/patterns/min_time_grain.py similarity index 100% rename from metricflow-semantics/metricflow_semantics/specs/patterns/base_time_grain.py rename to metricflow-semantics/metricflow_semantics/specs/patterns/min_time_grain.py diff --git a/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py b/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py index 4e2c32da35..d13b2bcd05 100644 --- a/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py +++ b/metricflow-semantics/metricflow_semantics/specs/query_param_implementations.py @@ -41,11 +41,6 @@ def _implements_protocol(self) -> TimeDimensionQueryParameter: grain: Optional[TimeGranularity] = None date_part: Optional[DatePart] = None - def __post_init__(self) -> None: # noqa: D105 - parsed_name = StructuredLinkableSpecName.from_name(self.name) - if parsed_name.time_granularity: - raise ValueError("Must use object syntax for `grain` parameter if `date_part` is requested.") - @property def query_resolver_input(self) -> ResolverInputForGroupByItem: # noqa: D102 fields_to_compare = [ @@ -53,7 +48,6 @@ def query_resolver_input(self) -> ResolverInputForGroupByItem: # noqa: D102 ParameterSetField.ENTITY_LINKS, ParameterSetField.DATE_PART, ] - # Is this left out because it needs to be resplved? If so document that. if self.grain is not None: fields_to_compare.append(ParameterSetField.TIME_GRANULARITY) diff --git a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py index 5adf2ec399..ce615ad1e4 100644 --- a/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py +++ b/metricflow-semantics/tests_metricflow_semantics/query/test_query_parser.py @@ -30,7 +30,7 @@ from metricflow_semantics.test_helpers.example_project_configuration import ( EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE, ) -from metricflow_semantics.test_helpers.metric_time_dimension import MTD, MTD_SPEC_DAY, MTD_SPEC_MONTH, MTD_SPEC_YEAR +from metricflow_semantics.test_helpers.metric_time_dimension import MTD, MTD_SPEC_MONTH, MTD_SPEC_YEAR from metricflow_semantics.test_helpers.snapshot_helpers import assert_object_snapshot_equal logger = logging.getLogger(__name__) @@ -76,7 +76,7 @@ - name: listing type: foreign expr: listing_id - + --- metric: name: instant_bookings