Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dedupe qualified name for GroupByMetricSpecs #1171

Merged
merged 5 commits into from
May 3, 2024
+924 −923
Merged

Dedupe qualified name for GroupByMetricSpecs #1171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b30af3c
Dedupe qualified name for GroupByMetricSpecs
courtneyholcomb Apr 30, 2024
0a276ce
Update SQL engine snapshots
courtneyholcomb Apr 30, 2024
bbd5d4a
Update snapshots properly
courtneyholcomb May 3, 2024
a2bc7fd
Always use spec's qualified name as column name
courtneyholcomb May 3, 2024
bf1312c
Update SQL engine snapshots
courtneyholcomb May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 5 additions & 21 deletions metricflow-semantics/metricflow_semantics/specs/dunder_column_association_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

from metricflow_semantics.model.semantic_manifest_lookup import SemanticManifestLookup
from metricflow_semantics.naming.linkable_spec_name import DUNDER, StructuredLinkableSpecName
from metricflow_semantics.naming.linkable_spec_name import DUNDER
from metricflow_semantics.specs.column_assoc import (
ColumnAssociation,
ColumnAssociationResolver,
Expand Down Expand Up @@ -59,23 +59,13 @@ def visit_measure_spec(self, measure_spec: MeasureSpec) -> ColumnAssociation: #

def visit_dimension_spec(self, dimension_spec: DimensionSpec) -> ColumnAssociation: # noqa: D102
return ColumnAssociation(
column_name=StructuredLinkableSpecName(
entity_link_names=tuple(x.element_name for x in dimension_spec.entity_links),
element_name=dimension_spec.element_name,
).qualified_name,
column_name=dimension_spec.qualified_name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I had just ripped this whole class out when I first noticed it none of this would've happened. Lesson learned. Thanks for making the property access nature of this class even more obvious. :)

single_column_correlation_key=SingleColumnCorrelationKey(),
)

def visit_time_dimension_spec(self, time_dimension_spec: TimeDimensionSpec) -> ColumnAssociation: # noqa: D102
column_name = StructuredLinkableSpecName(
entity_link_names=tuple(x.element_name for x in time_dimension_spec.entity_links),
element_name=time_dimension_spec.element_name,
time_granularity=time_dimension_spec.time_granularity,
date_part=time_dimension_spec.date_part,
).qualified_name

return ColumnAssociation(
column_name=column_name
column_name=time_dimension_spec.qualified_name
+ (
f"{DUNDER}{time_dimension_spec.aggregation_state.value.lower()}"
if time_dimension_spec.aggregation_state
Expand All @@ -86,19 +76,13 @@ def visit_time_dimension_spec(self, time_dimension_spec: TimeDimensionSpec) -> C

def visit_entity_spec(self, entity_spec: EntitySpec) -> ColumnAssociation: # noqa: D102
return ColumnAssociation(
column_name=StructuredLinkableSpecName(
entity_link_names=tuple(x.element_name for x in entity_spec.entity_links),
element_name=entity_spec.element_name,
).qualified_name,
column_name=entity_spec.qualified_name,
single_column_correlation_key=SingleColumnCorrelationKey(),
)

def visit_group_by_metric_spec(self, group_by_metric_spec: GroupByMetricSpec) -> ColumnAssociation: # noqa: D102
return ColumnAssociation(
column_name=StructuredLinkableSpecName(
entity_link_names=tuple(x.element_name for x in group_by_metric_spec.entity_links),
element_name=group_by_metric_spec.element_name,
).qualified_name,
column_name=group_by_metric_spec.qualified_name,
single_column_correlation_key=SingleColumnCorrelationKey(),
)

Expand Down
17 changes: 17 additions & 0 deletions metricflow-semantics/metricflow_semantics/specs/spec_classes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,23 @@ def metric_subquery_entity_spec(self) -> EntitySpec:
entity_links=self.metric_subquery_entity_links[:-1],
)

@property
def qualified_name(self) -> str:
"""Element name prefixed with entity links.
If same entity links are used in inner & outer query, use standard qualified name (country__bookings).
Else, specify both sets of entity links (listing__country__user__country__bookings).
"""
if self.entity_links == self.metric_subquery_entity_links:
entity_links = self.entity_links
else:
entity_links = self.entity_links + self.metric_subquery_entity_links

return StructuredLinkableSpecName(
entity_link_names=tuple(entity_link.element_name for entity_link in entity_links),
element_name=self.element_name,
).qualified_name

def __eq__(self, other: Any) -> bool: # type: ignore[misc] # noqa: D105
if not isinstance(other, GroupByMetricSpec):
return False
Expand Down
Loading
Loading