Skip to content

Commit

Permalink
Address comments for LinkableMetric path updates.
Browse files Browse the repository at this point in the history
  • Loading branch information
plypaul committed May 14, 2024
1 parent 729223d commit 6df9a6b
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,10 @@ def reference(self) -> GroupByMetricReference: # noqa: D102

@property
def join_by_semantic_model(self) -> Optional[SemanticModelReference]: # noqa: D102
return self.join_path.last_semantic_model_reference
if len(self.join_path.semantic_model_join_path.path_elements) == 0:
return None

return self.join_path.semantic_model_join_path.last_semantic_model_reference

@property
@override
Expand Down Expand Up @@ -349,7 +352,7 @@ class SemanticModelToMetricSubqueryJoinPath:
"""

metric_subquery_join_path_element: MetricSubqueryJoinPathElement
semantic_model_join_path: Optional[SemanticModelJoinPath] = None
semantic_model_join_path: SemanticModelJoinPath

@property
def entity_links(self) -> Tuple[EntityReference, ...]: # noqa: D102
Expand All @@ -362,8 +365,3 @@ def metric_subquery_entity_links(self) -> Tuple[EntityReference, ...]: # noqa:
return self.metric_subquery_join_path_element.entity_links + (
self.metric_subquery_join_path_element.join_on_entity,
)

@property
def last_semantic_model_reference(self) -> Optional[SemanticModelReference]:
"""The last semantic model that would be joined in this path (if exists) before joining to metric."""
return self.semantic_model_join_path.last_semantic_model_reference if self.semantic_model_join_path else None
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ def __init__(
for semantic_model in semantic_manifest.semantic_models:
linkable_element_sets_for_no_metrics_queries.append(self._get_elements_in_semantic_model(semantic_model))
linkable_element_sets_for_no_metrics_queries.append(
self.get_joinable_metrics_for_semantic_model(semantic_model)
self.get_joinable_metrics_for_semantic_model(
semantic_model, SemanticModelJoinPath(left_semantic_model_reference=semantic_model.reference)
)
)

metric_time_elements_for_no_metrics = self._get_metric_time_elements(measure_reference=None)
Expand Down Expand Up @@ -264,24 +266,32 @@ def _get_semantic_model_for_measure(self, measure_reference: MeasureReference) -
return semantic_models_where_measure_was_found[0]

def get_joinable_metrics_for_semantic_model(
self, semantic_model: SemanticModel, using_join_path: Optional[SemanticModelJoinPath] = None
self, semantic_model: SemanticModel, using_join_path: SemanticModelJoinPath
) -> LinkableElementSet:
"""Get the set of linkable metrics that can be joined to this semantic model."""
"""Get the set of linkable metrics that can be joined to this semantic model.
From @courtneyholcomb re: `using_join_path`:
That would be the join path to get from the outer query to the metric subquery. If you query metric1
filtered by metric2 (grouped by entity1), it would be the join path to get from metric1 to entity1. If
entity1 is local to the semantic model where metric1's input measures are defined, no join path is
necessary.
"""
properties = frozenset({LinkableElementProperty.METRIC, LinkableElementProperty.JOINED})
if using_join_path:

join_path_has_path_links = len(using_join_path.path_elements) > 0
if join_path_has_path_links:
assert semantic_model.reference == using_join_path.last_semantic_model_reference, (
"Last join path element should match semantic model when building LinkableMetrics. "
f"Got semantic model: {semantic_model.reference.semantic_model_name}; "
f"last join path element: {using_join_path.last_semantic_model_reference.semantic_model_name}",
)
properties = properties.union(frozenset({LinkableElementProperty.MULTI_HOP}))
# Temp: disable LinkableMetrics with outer join path until there is an interface to specify it.
return LinkableElementSet()

path_key_to_linkable_metrics: Dict[ElementPathKey, Tuple[LinkableMetric, ...]] = {}
for entity_reference in [entity.reference for entity in semantic_model.entities]:
# Avoid creating an entity link cycle.
if using_join_path and entity_reference in using_join_path.entity_links:
if join_path_has_path_links and entity_reference in using_join_path.entity_links:
continue
for metric_subquery_join_path_element in self._joinable_metrics_for_entities[entity_reference]:
linkable_metric = LinkableMetric(
Expand Down Expand Up @@ -564,7 +574,10 @@ def _get_linkable_element_set_for_measure(
measure_semantic_model = self._get_semantic_model_for_measure(measure_reference)

elements_in_semantic_model = self._get_elements_in_semantic_model(measure_semantic_model)
metrics_linked_to_semantic_model = self.get_joinable_metrics_for_semantic_model(measure_semantic_model)
metrics_linked_to_semantic_model = self.get_joinable_metrics_for_semantic_model(
semantic_model=measure_semantic_model,
using_join_path=SemanticModelJoinPath(left_semantic_model_reference=measure_semantic_model.reference),
)
metric_time_elements = self._get_metric_time_elements(measure_reference)
joined_elements = self._get_joined_elements(measure_semantic_model)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,14 @@ def assert_linkable_element_set_snapshot_equal( # noqa: D103

for linkable_metric_iterable in linkable_element_set.path_key_to_linkable_metrics.values():
for linkable_metric in linkable_metric_iterable:
semantic_model_join_path = linkable_metric.join_path.semantic_model_join_path
rows.append(
(
# Checking a limited set of fields as the result is large due to the paths in the object.
(
linkable_metric.join_by_semantic_model.semantic_model_name
if linkable_metric.join_by_semantic_model
else "",
(semantic_model_join_path.left_semantic_model_reference.semantic_model_name,)
+ tuple(
path_element.semantic_model_reference.semantic_model_name
for path_element in semantic_model_join_path.path_elements
),
(
str(tuple(entity_link.element_name for entity_link in linkable_metric.join_path.entity_links)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
time_granularity=None,
date_part=None,
)
# The opposite direction of the join tfor ambiguous_entity_with_join_path
# The opposite direction of the join for ambiguous_entity_with_join_path
# For testing deduplication on dimensions
_ambiguous_categorical_dimension_with_join_path = LinkableDimension(
element_name=AMBIGUOUS_NAME,
Expand Down Expand Up @@ -149,6 +149,7 @@
join_on_entity=_base_entity_reference,
entity_links=(),
),
semantic_model_join_path=SemanticModelJoinPath(left_semantic_model_reference=_base_semantic_model),
),
)
_ambiguous_metric = LinkableMetric(
Expand All @@ -160,6 +161,7 @@
join_on_entity=_base_entity_reference,
entity_links=(),
),
semantic_model_join_path=SemanticModelJoinPath(left_semantic_model_reference=_base_semantic_model),
),
)
# For testing deduplication on metrics
Expand All @@ -177,6 +179,7 @@
join_on_entity=_secondary_entity_reference,
),
),
semantic_model_join_path=SemanticModelJoinPath(left_semantic_model_reference=_base_semantic_model),
),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,10 @@ def test_metric_in_filter( # noqa: D103
derived_from_semantic_models=(SemanticModelReference("bookings"),),
join_on_entity=EntityReference("listing"),
entity_links=(),
)
),
semantic_model_join_path=SemanticModelJoinPath(
left_semantic_model_reference=SemanticModelReference("listings_latest")
),
),
),
)
Expand Down

0 comments on commit 6df9a6b

Please sign in to comment.