From 254130fb6ff8f7ec616f3d6b7c1cc168384e47e8 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Tue, 10 Oct 2023 17:17:36 -0700 Subject: [PATCH] Support dundered granularity syntax with object params --- metricflow/query/query_parser.py | 10 ++++----- .../specs/query_param_implementations.py | 6 ------ metricflow/test/query/test_query_parser.py | 21 ++++++++++++++----- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/metricflow/query/query_parser.py b/metricflow/query/query_parser.py index 6bb4185da2..cfd61462d6 100644 --- a/metricflow/query/query_parser.py +++ b/metricflow/query/query_parser.py @@ -518,7 +518,7 @@ def _validate_order_by_specs( or order_by_spec.instance_spec in linkable_specs.time_dimension_specs or order_by_spec.instance_spec in linkable_specs.entity_specs ): - raise InvalidQueryException(f"Order by item {order_by_spec} not in the query") + raise InvalidQueryException(f"Order by item {order_by_spec.instance_spec} not in the query") def _validate_date_part( self, metric_references: Sequence[MetricReference], time_dimension_specs: Sequence[TimeDimensionSpec] @@ -686,8 +686,8 @@ def _parse_group_by( entity_link_names=parsed_name.entity_link_names, element_name=parsed_name.element_name, time_granularity=group_by_obj.grain - if isinstance(group_by_obj, TimeDimensionQueryParameter) - else None, + if isinstance(group_by_obj, TimeDimensionQueryParameter) and group_by_obj.grain + else parsed_name.time_granularity, date_part=group_by_obj.date_part if isinstance(group_by_obj, TimeDimensionQueryParameter) else None, ) structured_names.append(structured_name) @@ -844,10 +844,8 @@ def _parse_order_by( descending = order.descending parsed_name = StructuredLinkableSpecName.from_name(order.order_by.name.lower()) if isinstance(order.order_by, TimeDimensionQueryParameter): - time_granularity = order.order_by.grain + time_granularity = order.order_by.grain or parsed_name.time_granularity date_part = order.order_by.date_part - if parsed_name.time_granularity and parsed_name.time_granularity != time_granularity: - raise InvalidQueryException("Must use object syntax to request a time granularity.") if MetricReference(element_name=parsed_name.element_name) in self._known_metric_names: if time_granularity: diff --git a/metricflow/specs/query_param_implementations.py b/metricflow/specs/query_param_implementations.py index 5b06b13f14..241d373e1e 100644 --- a/metricflow/specs/query_param_implementations.py +++ b/metricflow/specs/query_param_implementations.py @@ -5,7 +5,6 @@ from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity -from metricflow.naming.linkable_spec_name import StructuredLinkableSpecName from metricflow.protocols.query_parameter import InputOrderByParameter from metricflow.time.date_part import DatePart @@ -18,11 +17,6 @@ class TimeDimensionParameter: grain: Optional[TimeGranularity] = None date_part: Optional[DatePart] = None - def __post_init__(self) -> None: # noqa: D - 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.") - @dataclass(frozen=True) class DimensionOrEntityParameter: diff --git a/metricflow/test/query/test_query_parser.py b/metricflow/test/query/test_query_parser.py index 19ece527b4..decd9f2aa3 100644 --- a/metricflow/test/query/test_query_parser.py +++ b/metricflow/test/query/test_query_parser.py @@ -29,7 +29,7 @@ ) from metricflow.test.fixtures.model_fixtures import query_parser_from_yaml from metricflow.test.model.example_project_configuration import EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE -from metricflow.test.time.metric_time_dimension import MTD +from metricflow.test.time.metric_time_dimension import MTD, MTD_SPEC_MONTH from metricflow.time.date_part import DatePart from metricflow.time.time_granularity_solver import RequestTimeGranularityException @@ -256,11 +256,13 @@ def test_query_parser_with_object_params(bookings_query_parser: MetricFlowQueryP group_by = ( DimensionOrEntityParameter("booking__is_instant"), DimensionOrEntityParameter("listing"), - TimeDimensionParameter(MTD), + TimeDimensionParameter(MTD, date_part=DatePart.DOW), + TimeDimensionParameter(MTD_SPEC_MONTH.qualified_name), ) order_by = ( - OrderByParameter(order_by=TimeDimensionParameter(MTD)), + OrderByParameter(order_by=TimeDimensionParameter(MTD, date_part=DatePart.DOW)), OrderByParameter(order_by=MetricParameter("bookings"), descending=True), + OrderByParameter(order_by=TimeDimensionParameter(MTD_SPEC_MONTH.qualified_name), descending=True), ) query_spec = bookings_query_parser.parse_and_validate_query(metrics=[metric], group_by=group_by, order_by=order_by) assert query_spec.metric_specs == (MetricSpec(element_name="bookings"),) @@ -268,18 +270,27 @@ def test_query_parser_with_object_params(bookings_query_parser: MetricFlowQueryP DimensionSpec(element_name="is_instant", entity_links=(EntityReference("booking"),)), ) assert query_spec.time_dimension_specs == ( - TimeDimensionSpec(element_name=MTD, entity_links=(), time_granularity=TimeGranularity.DAY), + TimeDimensionSpec(element_name=MTD, entity_links=(), time_granularity=TimeGranularity.MONTH), + TimeDimensionSpec( + element_name=MTD, entity_links=(), time_granularity=TimeGranularity.DAY, date_part=DatePart.DOW + ), ) assert query_spec.entity_specs == (EntitySpec(element_name="listing", entity_links=()),) assert query_spec.order_by_specs == ( OrderBySpec( - instance_spec=TimeDimensionSpec(element_name=MTD, entity_links=(), time_granularity=TimeGranularity.DAY), + instance_spec=TimeDimensionSpec( + element_name=MTD, entity_links=(), time_granularity=TimeGranularity.DAY, date_part=DatePart.DOW + ), descending=False, ), OrderBySpec( instance_spec=MetricSpec(element_name="bookings"), descending=True, ), + OrderBySpec( + instance_spec=TimeDimensionSpec(element_name=MTD, entity_links=(), time_granularity=TimeGranularity.MONTH), + descending=True, + ), )