Skip to content

Commit

Permalink
Handle ambiguous metric_time filters in YAML definitions (#1342)
Browse files Browse the repository at this point in the history
Handle ambiguous `metric_time` filters in metric YAML definitions. The
expected behavior for an ambiguous `metric_time` filter in a metric's
YAML definition is that it will use the default time granularity for the
metric _where the filter is defined_. This means that if a filter is
defined on a derived metric's input metric, that filter will use the
derived metrics time granularity, not the input metric's time
granularity.
Aside from that change, this adds tests for other types of ambiguous
`metric_time` YAML filters.
  • Loading branch information
courtneyholcomb authored Jul 22, 2024
1 parent e2c2271 commit 80d4612
Show file tree
Hide file tree
Showing 29 changed files with 478 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Dict, FrozenSet, Iterator, List, Optional, Sequence, Tuple

from dbt_semantic_interfaces.enum_extension import assert_values_exhausted
from dbt_semantic_interfaces.references import MetricReference
from dbt_semantic_interfaces.type_enums import MetricType
from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity
from typing_extensions import override
Expand All @@ -15,6 +16,10 @@
from metricflow_semantics.model.linkable_element_property import LinkableElementProperty
from metricflow_semantics.model.semantic_manifest_lookup import SemanticManifestLookup
from metricflow_semantics.query.group_by_item.candidate_push_down.group_by_item_candidate import GroupByItemCandidateSet
from metricflow_semantics.query.group_by_item.filter_spec_resolution.filter_location import (
WhereFilterLocation,
WhereFilterLocationType,
)
from metricflow_semantics.query.group_by_item.resolution_dag.resolution_nodes.base_node import (
GroupByItemResolutionNode,
GroupByItemResolutionNodeVisitor,
Expand Down Expand Up @@ -135,6 +140,7 @@ def __init__(
source_spec_patterns: Sequence[SpecPattern] = (),
with_any_property: Optional[FrozenSet[LinkableElementProperty]] = None,
without_any_property: Optional[FrozenSet[LinkableElementProperty]] = None,
filter_location: Optional[WhereFilterLocation] = None,
) -> None:
"""Initializer.
Expand All @@ -146,13 +152,15 @@ def __init__(
with_any_property: Only consider group-by-items with these properties from the measure nodes.
without_any_property: Only consider group-by-items without any of these properties (see
LinkableElementProperty).
filter_location: If resolving a where filter item, where this filter was defined.
"""
self._semantic_manifest_lookup = manifest_lookup
self._source_spec_patterns = tuple(source_spec_patterns)
self._path_from_start_node_tracker = DagTraversalPathTracker()
self._with_any_property = with_any_property
self._without_any_property = without_any_property
self._suggestion_generator = suggestion_generator
self._filter_location = filter_location

@override
def visit_measure_node(self, node: MeasureGroupByItemSourceNode) -> PushDownResult:
Expand Down Expand Up @@ -369,11 +377,24 @@ def visit_metric_node(self, node: MetricGroupByItemResolutionNode) -> PushDownRe
)
)

metric_to_use_for_time_granularity_resolution = metric
# If this is resolving a filter defined on an input metric, use the outer metric's time_granularity.
if (
node.metric_input_location
and self._filter_location
and self._filter_location.location_type == WhereFilterLocationType.INPUT_METRIC
):
metric_to_use_for_time_granularity_resolution = self._semantic_manifest_lookup.metric_lookup.get_metric(
node.metric_input_location.derived_metric_reference
)

# If time granularity is not set for the metric, defaults to DAY if available, else the smallest available granularity.
# Note: ignores any granularity set on input metrics.
metric_default_time_granularity = metric.time_granularity or max(
metric_default_time_granularity = metric_to_use_for_time_granularity_resolution.time_granularity or max(
TimeGranularity.DAY,
self._semantic_manifest_lookup.metric_lookup.get_min_queryable_time_granularity(node.metric_reference),
self._semantic_manifest_lookup.metric_lookup.get_min_queryable_time_granularity(
MetricReference(metric_to_use_for_time_granularity_resolution.name)
),
)

if matched_items.spec_count == 0:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
from __future__ import annotations

from dataclasses import dataclass
from enum import Enum
from typing import Sequence, Tuple

from dbt_semantic_interfaces.references import MetricReference
from more_itertools import is_sorted


class WhereFilterLocationType(Enum):
"""Describes the location where the filter is defined."""

QUERY = "query"
METRIC = "metric"
INPUT_METRIC = "input_metric"


@dataclass(frozen=True)
class WhereFilterLocation:
"""Describe the location of a where filter.
When describing a query, the metric references are the metrics in the query.
When describing a metric, the metric reference is just the metric.
When describing a metric, the metric reference just the metric where the filter is defined.
When describing an input metric, the metric references are the outer metric and the input metric.
Since this is used to resolve valid group-by items for a where filter, this is sufficient as the valid group-by
items depend only on the metrics queried.
"""

location_type: WhereFilterLocationType
# These should be sorted for consistency in comparisons.
metric_references: Tuple[MetricReference, ...]

Expand All @@ -26,8 +37,16 @@ def __post_init__(self) -> None: # noqa: D105

@staticmethod
def for_query(metric_references: Sequence[MetricReference]) -> WhereFilterLocation: # noqa: D102
return WhereFilterLocation(metric_references=tuple(sorted(metric_references)))
return WhereFilterLocation(
metric_references=tuple(sorted(metric_references)), location_type=WhereFilterLocationType.QUERY
)

@staticmethod
def for_metric(metric_reference: MetricReference) -> WhereFilterLocation: # noqa: D102
return WhereFilterLocation(metric_references=(metric_reference,))
return WhereFilterLocation(metric_references=(metric_reference,), location_type=WhereFilterLocationType.METRIC)

@staticmethod
def for_input_metric(input_metric_reference: MetricReference) -> WhereFilterLocation: # noqa: D102
return WhereFilterLocation(
metric_references=(input_metric_reference,), location_type=WhereFilterLocationType.INPUT_METRIC
)
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
TimeDimensionCallParameterSet,
)
from dbt_semantic_interfaces.protocols import WhereFilterIntersection
from dbt_semantic_interfaces.references import MetricReference
from typing_extensions import override

from metricflow_semantics.collection_helpers.merger import Mergeable
Expand Down Expand Up @@ -163,28 +162,6 @@ def from_parameters( # noqa: D102
call_parameter_set=call_parameter_set,
)

@staticmethod
def for_metric_filter(
metric_reference: MetricReference, call_parameter_set: CallParameterSet
) -> ResolvedSpecLookUpKey:
"""Create a key related to a filter in a metric definition."""
return ResolvedSpecLookUpKey(
filter_location=WhereFilterLocation.for_metric(
metric_reference,
),
call_parameter_set=call_parameter_set,
)

@staticmethod
def for_query_filter(
metrics_in_query: Sequence[MetricReference], call_parameter_set: CallParameterSet
) -> ResolvedSpecLookUpKey:
"""Create a key related to a filter for a query."""
return ResolvedSpecLookUpKey(
filter_location=WhereFilterLocation.for_query(metrics_in_query),
call_parameter_set=call_parameter_set,
)


@dataclass(frozen=True)
class FilterSpecResolution:
Expand Down
Loading

0 comments on commit 80d4612

Please sign in to comment.