Skip to content

Commit

Permalink
Revert "Move _path_key_to_spec to ElementPathKey."
Browse files Browse the repository at this point in the history
This reverts commit bcbf1bf.

The original layout of linkable_element, with the private helper
converting ElementPathKey to the relevant LinkableInstanceSpec,
was intentionally placed to avoid circular dependencies with
the WhereFilterSpec during predicate pushdown implementation.

The code configuration from the reverted commit is nicer in many
respects, but unfortunately an attempt to use it ran into
circular dependencies as expected, and the TYPE_CHECKING work-around
ultimately failed because of the way SerializableDataclass works.

Since the SerializableDataclass mechanism is fully in use in the
implementation of some dbt cloud extensions we cannot easily
evalaute updates to that model here, and it is therefore much
simpler to just revert this change and move forward.
  • Loading branch information
tlento committed May 6, 2024
1 parent 7c580e1 commit 4bbc8e0
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@

from metricflow_semantics.model.linkable_element_property import LinkableElementProperty
from metricflow_semantics.model.semantic_model_derivation import SemanticModelDerivation
from metricflow_semantics.specs.spec_classes import (
DimensionSpec,
EntitySpec,
GroupByMetricSpec,
LinkableInstanceSpec,
TimeDimensionSpec,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -85,42 +78,6 @@ def __post_init__(self) -> None:
else:
assert_values_exhausted(element_type)

@property
def spec(self) -> LinkableInstanceSpec:
"""The corresponding spec object for this path key."""
if self.element_type is LinkableElementType.DIMENSION:
return DimensionSpec(
element_name=self.element_name,
entity_links=self.entity_links,
)
elif self.element_type is LinkableElementType.TIME_DIMENSION:
assert (
self.time_granularity is not None
), f"{self.time_granularity=} should not be None as per check in dataclass validation"
return TimeDimensionSpec(
element_name=self.element_name,
entity_links=self.entity_links,
time_granularity=self.time_granularity,
date_part=self.date_part,
)
elif self.element_type is LinkableElementType.ENTITY:
return EntitySpec(
element_name=self.element_name,
entity_links=self.entity_links,
)
elif self.element_type is LinkableElementType.METRIC:
assert self.metric_subquery_entity_links is not None, (
"ElementPathKeys for metrics must have non-null metric_subquery_entity_links."
"This should have been checked in post_init."
)
return GroupByMetricSpec(
element_name=self.element_name,
entity_links=self.entity_links,
metric_subquery_entity_links=self.metric_subquery_entity_links,
)
else:
assert_values_exhausted(self.element_type)


@dataclass(frozen=True)
class SemanticModelJoinPathElement:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from dataclasses import dataclass, field
from typing import Dict, FrozenSet, List, Sequence, Set, Tuple

from dbt_semantic_interfaces.enum_extension import assert_values_exhausted
from dbt_semantic_interfaces.references import SemanticModelReference
from typing_extensions import override

Expand All @@ -18,8 +19,12 @@
)
from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern
from metricflow_semantics.specs.spec_classes import (
DimensionSpec,
EntitySpec,
GroupByMetricSpec,
InstanceSpec,
LinkableInstanceSpec,
TimeDimensionSpec,
)


Expand Down Expand Up @@ -308,14 +313,50 @@ def spec_count(self) -> int:
@property
def specs(self) -> Sequence[LinkableInstanceSpec]:
"""Converts the items in a `LinkableElementSet` to their corresponding spec objects."""
return tuple(
path_key.spec
for path_key in (
tuple(self.path_key_to_linkable_dimensions.keys())
+ tuple(self.path_key_to_linkable_entities.keys())
+ tuple(self.path_key_to_linkable_metrics.keys())
specs: List[LinkableInstanceSpec] = []

for path_key in (
tuple(self.path_key_to_linkable_dimensions.keys())
+ tuple(self.path_key_to_linkable_entities.keys())
+ tuple(self.path_key_to_linkable_metrics.keys())
):
specs.append(LinkableElementSet._path_key_to_spec(path_key))

return specs

@staticmethod
def _path_key_to_spec(path_key: ElementPathKey) -> LinkableInstanceSpec:
"""Helper method to convert ElementPathKey instances to LinkableInstanceSpecs.
This is currently used in the context of switching between ElementPathKeys and LinkableInstanceSpecs
within a LinkableElementSet, so we leave it here for now.
"""
if path_key.element_type is LinkableElementType.DIMENSION:
return DimensionSpec(
element_name=path_key.element_name,
entity_links=path_key.entity_links,
)
)
elif path_key.element_type is LinkableElementType.TIME_DIMENSION:
assert path_key.time_granularity is not None
return TimeDimensionSpec(
element_name=path_key.element_name,
entity_links=path_key.entity_links,
time_granularity=path_key.time_granularity,
date_part=path_key.date_part,
)
elif path_key.element_type is LinkableElementType.ENTITY:
return EntitySpec(
element_name=path_key.element_name,
entity_links=path_key.entity_links,
)
elif path_key.element_type is LinkableElementType.METRIC:
return GroupByMetricSpec(
element_name=path_key.element_name,
entity_links=path_key.entity_links,
metric_subquery_entity_links=path_key.metric_subquery_entity_links,
)
else:
assert_values_exhausted(path_key.element_type)

def filter_by_spec_patterns(self, spec_patterns: Sequence[SpecPattern]) -> LinkableElementSet:
"""Filter the elements in the set by the given spec patters.
Expand All @@ -335,15 +376,15 @@ def filter_by_spec_patterns(self, spec_patterns: Sequence[SpecPattern]) -> Linka
path_key_to_linkable_metrics: Dict[ElementPathKey, Tuple[LinkableMetric, ...]] = {}

for path_key, linkable_dimensions in self.path_key_to_linkable_dimensions.items():
if path_key.spec in specs_to_include:
if LinkableElementSet._path_key_to_spec(path_key) in specs_to_include:
path_key_to_linkable_dimensions[path_key] = linkable_dimensions

for path_key, linkable_entities in self.path_key_to_linkable_entities.items():
if path_key.spec in specs_to_include:
if LinkableElementSet._path_key_to_spec(path_key) in specs_to_include:
path_key_to_linkable_entities[path_key] = linkable_entities

for path_key, linkable_metrics in self.path_key_to_linkable_metrics.items():
if path_key.spec in specs_to_include:
if LinkableElementSet._path_key_to_spec(path_key) in specs_to_include:
path_key_to_linkable_metrics[path_key] = linkable_metrics

return LinkableElementSet(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from metricflow_semantics.errors.error_classes import UnableToSatisfyQueryError
from metricflow_semantics.filters.time_constraint import TimeRangeConstraint
from metricflow_semantics.model.linkable_element_property import LinkableElementProperty
from metricflow_semantics.model.semantics.linkable_element_set import LinkableElementSet
from metricflow_semantics.query.query_parser import MetricFlowQueryParser
from metricflow_semantics.specs.column_assoc import ColumnAssociationResolver
from metricflow_semantics.specs.query_spec import MetricFlowQuerySpec
Expand Down Expand Up @@ -1338,7 +1339,7 @@ def test_all_available_single_join_metric_filters(
MeasureReference("listings"), without_any_of={LinkableElementProperty.MULTI_HOP}
).path_key_to_linkable_metrics.values():
for linkable_metric in linkable_metric_tuple:
group_by_metric_spec = linkable_metric.path_key.spec
group_by_metric_spec = LinkableElementSet._path_key_to_spec(linkable_metric.path_key)
assert isinstance(group_by_metric_spec, GroupByMetricSpec)
entity_spec = group_by_metric_spec.metric_subquery_entity_spec
if entity_spec.entity_links: # multi-hop for inner query
Expand Down

0 comments on commit 4bbc8e0

Please sign in to comment.