Support dundered granularity syntax with object params

dbt-labs · Oct 11, 2023 · 254130f · 254130f
1 parent 81ae014
commit 254130f
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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,30 +256,41 @@ 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"),)
     assert query_spec.dimension_specs == (
         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,
+        ),
     )