From 735611e84525971699e2bea640d9e83241ccf852 Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Mon, 18 Sep 2023 13:23:57 -0500 Subject: [PATCH 1/7] added support for descending --- metricflow/engine/metricflow_engine.py | 22 ++++++------ metricflow/errors/errors.py | 7 ++++ metricflow/query/query_parser.py | 22 ++++++------ metricflow/specs/query_interface.py | 34 +++++++----------- metricflow/specs/query_parameter.py | 36 +++++++++++++++++++ metricflow/specs/where_filter_dimension.py | 7 ++++ metricflow/specs/where_filter_entity.py | 7 ++++ .../specs/where_filter_time_dimension.py | 11 +++++- 8 files changed, 102 insertions(+), 44 deletions(-) create mode 100644 metricflow/specs/query_parameter.py diff --git a/metricflow/engine/metricflow_engine.py b/metricflow/engine/metricflow_engine.py index 1d459bca78..33ab20bcc1 100644 --- a/metricflow/engine/metricflow_engine.py +++ b/metricflow/engine/metricflow_engine.py @@ -52,7 +52,7 @@ from metricflow.query.query_parser import MetricFlowQueryParser from metricflow.random_id import random_id from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import QueryInterfaceMetric, QueryParameter +from metricflow.specs.query_interface import QueryParameterMetric, QueryParameterDimension from metricflow.specs.specs import InstanceSpecSet, MetricFlowQuerySpec from metricflow.sql.optimizer.optimization_levels import SqlQueryOptimizationLevel from metricflow.telemetry.models import TelemetryLevel @@ -98,15 +98,15 @@ class MetricFlowQueryRequest: request_id: MetricFlowRequestId metric_names: Optional[Sequence[str]] = None - metrics: Optional[Sequence[QueryInterfaceMetric]] = None + metrics: Optional[Sequence[QueryParameterMetric]] = None group_by_names: Optional[Sequence[str]] = None - group_by: Optional[Sequence[QueryParameter]] = None + group_by: Optional[Sequence[QueryParameterDimension]] = None limit: Optional[int] = None time_constraint_start: Optional[datetime.datetime] = None time_constraint_end: Optional[datetime.datetime] = None where_constraint: Optional[str] = None order_by_names: Optional[Sequence[str]] = None - order_by: Optional[Sequence[QueryParameter]] = None + order_by: Optional[Sequence[QueryParameterDimension]] = None output_table: Optional[str] = None sql_optimization_level: SqlQueryOptimizationLevel = SqlQueryOptimizationLevel.O4 query_type: MetricFlowQueryType = MetricFlowQueryType.METRIC @@ -114,15 +114,15 @@ class MetricFlowQueryRequest: @staticmethod def create_with_random_request_id( # noqa: D metric_names: Optional[Sequence[str]] = None, - metrics: Optional[Sequence[QueryInterfaceMetric]] = None, + metrics: Optional[Sequence[QueryParameterMetric]] = None, group_by_names: Optional[Sequence[str]] = None, - group_by: Optional[Sequence[QueryParameter]] = None, + group_by: Optional[Sequence[QueryParameterDimension]] = None, limit: Optional[int] = None, time_constraint_start: Optional[datetime.datetime] = None, time_constraint_end: Optional[datetime.datetime] = None, where_constraint: Optional[str] = None, order_by_names: Optional[Sequence[str]] = None, - order_by: Optional[Sequence[QueryParameter]] = None, + order_by: Optional[Sequence[QueryParameterDimension]] = None, output_table: Optional[str] = None, sql_optimization_level: SqlQueryOptimizationLevel = SqlQueryOptimizationLevel.O4, query_type: MetricFlowQueryType = MetricFlowQueryType.METRIC, @@ -286,9 +286,9 @@ def get_dimension_values( def explain_get_dimension_values( # noqa: D self, metric_names: Optional[List[str]] = None, - metrics: Optional[Sequence[QueryInterfaceMetric]] = None, + metrics: Optional[Sequence[QueryParameterMetric]] = None, get_group_by_values: Optional[str] = None, - group_by: Optional[QueryParameter] = None, + group_by: Optional[QueryParameterDimension] = None, time_constraint_start: Optional[datetime.datetime] = None, time_constraint_end: Optional[datetime.datetime] = None, ) -> MetricFlowExplainResult: @@ -681,9 +681,9 @@ def get_dimension_values( # noqa: D def explain_get_dimension_values( # noqa: D self, metric_names: Optional[List[str]] = None, - metrics: Optional[Sequence[QueryInterfaceMetric]] = None, + metrics: Optional[Sequence[QueryParameterMetric]] = None, get_group_by_values: Optional[str] = None, - group_by: Optional[QueryParameter] = None, + group_by: Optional[QueryParameterDimension] = None, time_constraint_start: Optional[datetime.datetime] = None, time_constraint_end: Optional[datetime.datetime] = None, ) -> MetricFlowExplainResult: diff --git a/metricflow/errors/errors.py b/metricflow/errors/errors.py index d27b0e01dc..bddb576e8b 100644 --- a/metricflow/errors/errors.py +++ b/metricflow/errors/errors.py @@ -75,3 +75,10 @@ class SqlBindParametersNotSupportedError(Exception): class UnknownMetricLinkingError(ValueError): """Raised during linking when a user attempts to use a metric that isn't specified.""" + + +class InvalidQuerySyntax(Exception): + """Raised when query syntax is invalid. Primarily used in the where clause.""" + + def __init__(self, msg: str) -> None: # noqa: D + super().__init__(msg) diff --git a/metricflow/query/query_parser.py b/metricflow/query/query_parser.py index 0027e68322..04c647615c 100644 --- a/metricflow/query/query_parser.py +++ b/metricflow/query/query_parser.py @@ -30,7 +30,7 @@ from metricflow.naming.linkable_spec_name import StructuredLinkableSpecName from metricflow.query.query_exceptions import InvalidQueryException from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import QueryInterfaceMetric, QueryParameter +from metricflow.specs.query_interface import QueryParameterMetric, QueryParameterDimension from metricflow.specs.specs import ( DimensionSpec, EntitySpec, @@ -169,16 +169,16 @@ def _top_fuzzy_matches( def parse_and_validate_query( self, metric_names: Optional[Sequence[str]] = None, - metrics: Optional[Sequence[QueryInterfaceMetric]] = None, + metrics: Optional[Sequence[QueryParameterMetric]] = None, group_by_names: Optional[Sequence[str]] = None, - group_by: Optional[Sequence[QueryParameter]] = None, + group_by: Optional[Sequence[QueryParameterDimension]] = None, limit: Optional[int] = None, time_constraint_start: Optional[datetime.datetime] = None, time_constraint_end: Optional[datetime.datetime] = None, where_constraint: Optional[WhereFilter] = None, where_constraint_str: Optional[str] = None, order: Optional[Sequence[str]] = None, - order_by: Optional[Sequence[QueryParameter]] = None, + order_by: Optional[Sequence[QueryParameterDimension]] = None, time_granularity: Optional[TimeGranularity] = None, ) -> MetricFlowQuerySpec: """Parse the query into spec objects, validating them in the process. @@ -290,7 +290,7 @@ def _construct_metric_specs_for_query( return tuple(metric_specs) def _get_group_by_names( - self, group_by_names: Optional[Sequence[str]], group_by: Optional[Sequence[QueryParameter]] + self, group_by_names: Optional[Sequence[str]], group_by: Optional[Sequence[QueryParameterDimension]] ) -> Sequence[str]: assert not ( group_by_names and group_by @@ -304,7 +304,7 @@ def _get_group_by_names( ) def _get_metric_names( - self, metric_names: Optional[Sequence[str]], metrics: Optional[Sequence[QueryInterfaceMetric]] + self, metric_names: Optional[Sequence[str]], metrics: Optional[Sequence[QueryParameterMetric]] ) -> Sequence[str]: assert_exactly_one_arg_set(metric_names=metric_names, metrics=metrics) return metric_names if metric_names else [m.name for m in metrics] if metrics else [] @@ -321,7 +321,9 @@ def _get_where_filter( PydanticWhereFilter(where_sql_template=where_constraint_str) if where_constraint_str else where_constraint ) - def _get_order(self, order: Optional[Sequence[str]], order_by: Optional[Sequence[QueryParameter]]) -> Sequence[str]: + def _get_order( + self, order: Optional[Sequence[str]], order_by: Optional[Sequence[QueryParameterDimension]] + ) -> Sequence[str]: assert not ( order and order_by ), "Both order_by_names and order_by were set, but if an order by is specified you should only use one of these!" @@ -330,16 +332,16 @@ def _get_order(self, order: Optional[Sequence[str]], order_by: Optional[Sequence def _parse_and_validate_query( self, metric_names: Optional[Sequence[str]] = None, - metrics: Optional[Sequence[QueryInterfaceMetric]] = None, + metrics: Optional[Sequence[QueryParameterMetric]] = None, group_by_names: Optional[Sequence[str]] = None, - group_by: Optional[Sequence[QueryParameter]] = None, + group_by: Optional[Sequence[QueryParameterDimension]] = None, limit: Optional[int] = None, time_constraint_start: Optional[datetime.datetime] = None, time_constraint_end: Optional[datetime.datetime] = None, where_constraint: Optional[WhereFilter] = None, where_constraint_str: Optional[str] = None, order: Optional[Sequence[str]] = None, - order_by: Optional[Sequence[QueryParameter]] = None, + order_by: Optional[Sequence[QueryParameterDimension]] = None, time_granularity: Optional[TimeGranularity] = None, ) -> MetricFlowQuerySpec: metric_names = self._get_metric_names(metric_names, metrics) diff --git a/metricflow/specs/query_interface.py b/metricflow/specs/query_interface.py index ee55a52086..44caa4673a 100644 --- a/metricflow/specs/query_interface.py +++ b/metricflow/specs/query_interface.py @@ -1,30 +1,13 @@ from __future__ import annotations -from typing import Optional, Protocol, Sequence - -from dbt_semantic_interfaces.type_enums import TimeGranularity +from typing import Protocol, Sequence class QueryInterfaceMetric(Protocol): - """Metric in the query interface.""" - - @property - def name(self) -> str: - """The name of the metric.""" - raise NotImplementedError - - -class QueryParameter(Protocol): - """A query parameter with a grain.""" - - @property - def name(self) -> str: - """The name of the item.""" - raise NotImplementedError + """Represents the interface for Metric in the query interface.""" - @property - def grain(self) -> Optional[TimeGranularity]: - """The time granularity.""" + def descending(self, _is_descending: bool) -> QueryInterfaceDimension: + """Set the sort order for order-by.""" raise NotImplementedError @@ -39,6 +22,10 @@ def alias(self, _alias: str) -> QueryInterfaceDimension: """Renaming the column.""" raise NotImplementedError + def descending(self, _is_descending: bool) -> QueryInterfaceDimension: + """Set the sort order for order-by.""" + raise NotImplementedError + class QueryInterfaceDimensionFactory(Protocol): """Creates a Dimension for the query interface. @@ -67,6 +54,7 @@ def create( self, time_dimension_name: str, time_granularity_name: str, + descending: bool = False, entity_path: Sequence[str] = (), ) -> QueryInterfaceTimeDimension: """Create a TimeDimension.""" @@ -76,7 +64,9 @@ def create( class QueryInterfaceEntity(Protocol): """Represents the interface for Entity in the query interface.""" - pass + def descending(self, _is_descending: bool) -> QueryInterfaceDimension: + """Set the sort order for order-by.""" + raise NotImplementedError class QueryInterfaceEntityFactory(Protocol): diff --git a/metricflow/specs/query_parameter.py b/metricflow/specs/query_parameter.py new file mode 100644 index 0000000000..8235ac2a7a --- /dev/null +++ b/metricflow/specs/query_parameter.py @@ -0,0 +1,36 @@ +from typing import Optional, Protocol + +from dbt_semantic_interfaces.type_enums import TimeGranularity + + +class QueryParameterDimension(Protocol): + """A query parameter with a grain.""" + + @property + def name(self) -> str: + """The name of the item.""" + raise NotImplementedError + + @property + def grain(self) -> Optional[TimeGranularity]: + """The time granularity.""" + raise NotImplementedError + + @property + def descending(self) -> bool: + """Set the sort order for order-by.""" + raise NotImplementedError + + +class QueryParameterMetric(Protocol): + """Metric in the query interface.""" + + @property + def name(self) -> str: + """The name of the metric.""" + raise NotImplementedError + + @property + def descending(self) -> bool: + """Set the sort order for order-by.""" + raise NotImplementedError diff --git a/metricflow/specs/where_filter_dimension.py b/metricflow/specs/where_filter_dimension.py index 3f33f87d96..e6dff04c94 100644 --- a/metricflow/specs/where_filter_dimension.py +++ b/metricflow/specs/where_filter_dimension.py @@ -13,6 +13,7 @@ EntityReference, ) from typing_extensions import override +from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.query_interface import ( @@ -40,6 +41,12 @@ def alias(self, _alias: str) -> QueryInterfaceDimension: """Renaming the column.""" raise NotImplementedError + def descending(self, _is_descending: bool) -> QueryInterfaceDimension: + """Set the sort order for order-by.""" + raise InvalidQuerySyntax( + "Can't set descending in the where clause. Try setting descending in the order_by clause instead" + ) + def __str__(self) -> str: """Returns the column name. diff --git a/metricflow/specs/where_filter_entity.py b/metricflow/specs/where_filter_entity.py index 83c983fd39..230e1c28c2 100644 --- a/metricflow/specs/where_filter_entity.py +++ b/metricflow/specs/where_filter_entity.py @@ -10,6 +10,7 @@ from dbt_semantic_interfaces.protocols.protocol_hint import ProtocolHint from dbt_semantic_interfaces.references import EntityReference from typing_extensions import override +from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.query_interface import QueryInterfaceEntity, QueryInterfaceEntityFactory @@ -26,6 +27,12 @@ def _implements_protocol(self) -> QueryInterfaceEntity: def __init__(self, column_name: str): # noqa self.column_name = column_name + def descending(self, _is_descending: bool) -> QueryInterfaceEntity: + """Set the sort order for order-by.""" + raise InvalidQuerySyntax( + "Can't set descending in the where clause. Try setting descending in the order_by clause instead" + ) + def __str__(self) -> str: """Returns the column name. diff --git a/metricflow/specs/where_filter_time_dimension.py b/metricflow/specs/where_filter_time_dimension.py index 9ad0bb1826..8630465f6b 100644 --- a/metricflow/specs/where_filter_time_dimension.py +++ b/metricflow/specs/where_filter_time_dimension.py @@ -8,6 +8,7 @@ from dbt_semantic_interfaces.references import EntityReference, TimeDimensionReference from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity from typing_extensions import override +from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.query_interface import QueryInterfaceTimeDimension, QueryInterfaceTimeDimensionFactory @@ -52,9 +53,17 @@ def __init__( # noqa self.time_dimension_specs: List[TimeDimensionSpec] = [] def create( - self, time_dimension_name: str, time_granularity_name: str, entity_path: Sequence[str] = () + self, + time_dimension_name: str, + time_granularity_name: str, + descending: bool = False, + entity_path: Sequence[str] = (), ) -> WhereFilterTimeDimension: """Create a WhereFilterTimeDimension.""" + if descending: + raise InvalidQuerySyntax( + "Can't set descending in the where clause. Try setting descending in the order_by clause instead" + ) structured_name = DunderedNameFormatter.parse_name(time_dimension_name) call_parameter_set = TimeDimensionCallParameterSet( time_dimension_reference=TimeDimensionReference(element_name=structured_name.element_name), From 4e97c113a4986a76cf8a0ef6257cdb88eb0ad197 Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Mon, 18 Sep 2023 14:06:52 -0500 Subject: [PATCH 2/7] added tests --- metricflow/engine/metricflow_engine.py | 2 +- metricflow/query/query_parser.py | 2 +- .../test/specs/test_where_filter_dimension.py | 8 ++++++ .../test/specs/test_where_filter_entity.py | 8 ++++++ .../specs/test_where_filter_time_dimension.py | 25 +++++++++++++++++++ 5 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 metricflow/test/specs/test_where_filter_dimension.py create mode 100644 metricflow/test/specs/test_where_filter_entity.py create mode 100644 metricflow/test/specs/test_where_filter_time_dimension.py diff --git a/metricflow/engine/metricflow_engine.py b/metricflow/engine/metricflow_engine.py index 33ab20bcc1..7477066233 100644 --- a/metricflow/engine/metricflow_engine.py +++ b/metricflow/engine/metricflow_engine.py @@ -52,7 +52,7 @@ from metricflow.query.query_parser import MetricFlowQueryParser from metricflow.random_id import random_id from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import QueryParameterMetric, QueryParameterDimension +from metricflow.specs.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.specs.specs import InstanceSpecSet, MetricFlowQuerySpec from metricflow.sql.optimizer.optimization_levels import SqlQueryOptimizationLevel from metricflow.telemetry.models import TelemetryLevel diff --git a/metricflow/query/query_parser.py b/metricflow/query/query_parser.py index 04c647615c..ac68085457 100644 --- a/metricflow/query/query_parser.py +++ b/metricflow/query/query_parser.py @@ -30,7 +30,7 @@ from metricflow.naming.linkable_spec_name import StructuredLinkableSpecName from metricflow.query.query_exceptions import InvalidQueryException from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import QueryParameterMetric, QueryParameterDimension +from metricflow.specs.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.specs.specs import ( DimensionSpec, EntitySpec, diff --git a/metricflow/test/specs/test_where_filter_dimension.py b/metricflow/test/specs/test_where_filter_dimension.py new file mode 100644 index 0000000000..0480e8c206 --- /dev/null +++ b/metricflow/test/specs/test_where_filter_dimension.py @@ -0,0 +1,8 @@ +import pytest +from metricflow.errors.errors import InvalidQuerySyntax +from metricflow.specs.where_filter_dimension import WhereFilterDimension + + +def test_descending_cannot_be_set(): + with pytest.raises(InvalidQuerySyntax): + WhereFilterDimension("bookings").descending(True) diff --git a/metricflow/test/specs/test_where_filter_entity.py b/metricflow/test/specs/test_where_filter_entity.py new file mode 100644 index 0000000000..a3b1fe5406 --- /dev/null +++ b/metricflow/test/specs/test_where_filter_entity.py @@ -0,0 +1,8 @@ +import pytest +from metricflow.errors.errors import InvalidQuerySyntax +from metricflow.specs.where_filter_entity import WhereFilterEntity + + +def test_descending_cannot_be_set(): + with pytest.raises(InvalidQuerySyntax): + WhereFilterEntity("customer").descending(True) diff --git a/metricflow/test/specs/test_where_filter_time_dimension.py b/metricflow/test/specs/test_where_filter_time_dimension.py new file mode 100644 index 0000000000..c49a3c066d --- /dev/null +++ b/metricflow/test/specs/test_where_filter_time_dimension.py @@ -0,0 +1,25 @@ +from abc import abstractmethod +from typing_extensions import override +import pytest +from metricflow.errors.errors import InvalidQuerySyntax +from metricflow.specs.where_filter_time_dimension import WhereFilterTimeDimensionFactory +from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets +from metricflow.specs.column_assoc import ColumnAssociationResolver +from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity +from metricflow.specs.specs import InstanceSpec +from metricflow.specs.column_assoc import ColumnAssociation + + +class MockColumnAssociationResolver(ColumnAssociationResolver): + """Just a mock for testing.""" + + @abstractmethod + def resolve_spec(self, spec: InstanceSpec) -> ColumnAssociation: # noqa: D + raise NotImplementedError + + +def test_descending_cannot_be_set(): # noqa + with pytest.raises(InvalidQuerySyntax): + WhereFilterTimeDimensionFactory(FilterCallParameterSets(), MockColumnAssociationResolver).create( + "metric_time", TimeGranularity.WEEK, descending=True + ) From 9d5abc65b39a4a21ddceb93d5714b8f174c2b6d3 Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Mon, 18 Sep 2023 15:55:32 -0500 Subject: [PATCH 3/7] changie --- .changes/unreleased/Features-20230918-155524.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20230918-155524.yaml diff --git a/.changes/unreleased/Features-20230918-155524.yaml b/.changes/unreleased/Features-20230918-155524.yaml new file mode 100644 index 0000000000..d6d4124074 --- /dev/null +++ b/.changes/unreleased/Features-20230918-155524.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Support for sort order in query interface +time: 2023-09-18T15:55:24.086263-05:00 +custom: + Author: DevonFulcher + Issue: None From aafc41853b7fd05d3be0beb1617f49e48f699be2 Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Mon, 18 Sep 2023 16:14:10 -0500 Subject: [PATCH 4/7] fixed linting & test --- metricflow/specs/query_interface.py | 4 +-- metricflow/specs/query_parameter.py | 2 ++ metricflow/specs/where_filter_dimension.py | 2 +- metricflow/specs/where_filter_entity.py | 2 +- .../specs/where_filter_time_dimension.py | 2 +- metricflow/test/query/test_query_parser.py | 5 ++-- .../test/specs/test_where_filter_dimension.py | 5 +++- .../test/specs/test_where_filter_entity.py | 5 +++- .../specs/test_where_filter_time_dimension.py | 25 ------------------- 9 files changed, 18 insertions(+), 34 deletions(-) delete mode 100644 metricflow/test/specs/test_where_filter_time_dimension.py diff --git a/metricflow/specs/query_interface.py b/metricflow/specs/query_interface.py index 44caa4673a..0ed6223489 100644 --- a/metricflow/specs/query_interface.py +++ b/metricflow/specs/query_interface.py @@ -6,7 +6,7 @@ class QueryInterfaceMetric(Protocol): """Represents the interface for Metric in the query interface.""" - def descending(self, _is_descending: bool) -> QueryInterfaceDimension: + def descending(self, _is_descending: bool) -> QueryInterfaceMetric: """Set the sort order for order-by.""" raise NotImplementedError @@ -64,7 +64,7 @@ def create( class QueryInterfaceEntity(Protocol): """Represents the interface for Entity in the query interface.""" - def descending(self, _is_descending: bool) -> QueryInterfaceDimension: + def descending(self, _is_descending: bool) -> QueryInterfaceEntity: """Set the sort order for order-by.""" raise NotImplementedError diff --git a/metricflow/specs/query_parameter.py b/metricflow/specs/query_parameter.py index 8235ac2a7a..4df99cc7f7 100644 --- a/metricflow/specs/query_parameter.py +++ b/metricflow/specs/query_parameter.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from typing import Optional, Protocol from dbt_semantic_interfaces.type_enums import TimeGranularity diff --git a/metricflow/specs/where_filter_dimension.py b/metricflow/specs/where_filter_dimension.py index e6dff04c94..4e2cc1933c 100644 --- a/metricflow/specs/where_filter_dimension.py +++ b/metricflow/specs/where_filter_dimension.py @@ -13,8 +13,8 @@ EntityReference, ) from typing_extensions import override -from metricflow.errors.errors import InvalidQuerySyntax +from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.query_interface import ( QueryInterfaceDimension, diff --git a/metricflow/specs/where_filter_entity.py b/metricflow/specs/where_filter_entity.py index 230e1c28c2..f935384493 100644 --- a/metricflow/specs/where_filter_entity.py +++ b/metricflow/specs/where_filter_entity.py @@ -10,8 +10,8 @@ from dbt_semantic_interfaces.protocols.protocol_hint import ProtocolHint from dbt_semantic_interfaces.references import EntityReference from typing_extensions import override -from metricflow.errors.errors import InvalidQuerySyntax +from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.query_interface import QueryInterfaceEntity, QueryInterfaceEntityFactory from metricflow.specs.specs import EntitySpec diff --git a/metricflow/specs/where_filter_time_dimension.py b/metricflow/specs/where_filter_time_dimension.py index 8630465f6b..0d8567788c 100644 --- a/metricflow/specs/where_filter_time_dimension.py +++ b/metricflow/specs/where_filter_time_dimension.py @@ -8,8 +8,8 @@ from dbt_semantic_interfaces.references import EntityReference, TimeDimensionReference from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity from typing_extensions import override -from metricflow.errors.errors import InvalidQuerySyntax +from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.query_interface import QueryInterfaceTimeDimension, QueryInterfaceTimeDimensionFactory from metricflow.specs.specs import TimeDimensionSpec diff --git a/metricflow/test/query/test_query_parser.py b/metricflow/test/query/test_query_parser.py index d585ab4465..4453ef565e 100644 --- a/metricflow/test/query/test_query_parser.py +++ b/metricflow/test/query/test_query_parser.py @@ -175,14 +175,15 @@ class MockQueryParameter: """This is a mock that is just used to test the query parser.""" grain = None + descending = False def __init__(self, name: str): # noqa: D self.name = name def test_query_parser_with_object_params(bookings_query_parser: MetricFlowQueryParser) -> None: # noqa: D - Metric = namedtuple("Metric", ["name"]) - metric = Metric("bookings") + Metric = namedtuple("Metric", ["name", "descending"]) + metric = Metric("bookings", False) group_by = [ MockQueryParameter("booking__is_instant"), MockQueryParameter("listing"), diff --git a/metricflow/test/specs/test_where_filter_dimension.py b/metricflow/test/specs/test_where_filter_dimension.py index 0480e8c206..a3c673daae 100644 --- a/metricflow/test/specs/test_where_filter_dimension.py +++ b/metricflow/test/specs/test_where_filter_dimension.py @@ -1,8 +1,11 @@ +from __future__ import annotations + import pytest + from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.where_filter_dimension import WhereFilterDimension -def test_descending_cannot_be_set(): +def test_descending_cannot_be_set() -> None: # noqa with pytest.raises(InvalidQuerySyntax): WhereFilterDimension("bookings").descending(True) diff --git a/metricflow/test/specs/test_where_filter_entity.py b/metricflow/test/specs/test_where_filter_entity.py index a3b1fe5406..ffbc3ad0a8 100644 --- a/metricflow/test/specs/test_where_filter_entity.py +++ b/metricflow/test/specs/test_where_filter_entity.py @@ -1,8 +1,11 @@ +from __future__ import annotations + import pytest + from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.where_filter_entity import WhereFilterEntity -def test_descending_cannot_be_set(): +def test_descending_cannot_be_set() -> None: # noqa with pytest.raises(InvalidQuerySyntax): WhereFilterEntity("customer").descending(True) diff --git a/metricflow/test/specs/test_where_filter_time_dimension.py b/metricflow/test/specs/test_where_filter_time_dimension.py deleted file mode 100644 index c49a3c066d..0000000000 --- a/metricflow/test/specs/test_where_filter_time_dimension.py +++ /dev/null @@ -1,25 +0,0 @@ -from abc import abstractmethod -from typing_extensions import override -import pytest -from metricflow.errors.errors import InvalidQuerySyntax -from metricflow.specs.where_filter_time_dimension import WhereFilterTimeDimensionFactory -from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets -from metricflow.specs.column_assoc import ColumnAssociationResolver -from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity -from metricflow.specs.specs import InstanceSpec -from metricflow.specs.column_assoc import ColumnAssociation - - -class MockColumnAssociationResolver(ColumnAssociationResolver): - """Just a mock for testing.""" - - @abstractmethod - def resolve_spec(self, spec: InstanceSpec) -> ColumnAssociation: # noqa: D - raise NotImplementedError - - -def test_descending_cannot_be_set(): # noqa - with pytest.raises(InvalidQuerySyntax): - WhereFilterTimeDimensionFactory(FilterCallParameterSets(), MockColumnAssociationResolver).create( - "metric_time", TimeGranularity.WEEK, descending=True - ) From dda5ce3b1d38c7ef80442a93b73fe83383cf515f Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Wed, 20 Sep 2023 09:46:07 -0500 Subject: [PATCH 5/7] moved query interface and query parameters into protocols folder --- metricflow/engine/metricflow_engine.py | 2 +- metricflow/{specs => protocols}/query_interface.py | 0 metricflow/{specs => protocols}/query_parameter.py | 0 metricflow/query/query_parser.py | 2 +- metricflow/specs/where_filter_dimension.py | 2 +- metricflow/specs/where_filter_entity.py | 2 +- metricflow/specs/where_filter_time_dimension.py | 2 +- 7 files changed, 5 insertions(+), 5 deletions(-) rename metricflow/{specs => protocols}/query_interface.py (100%) rename metricflow/{specs => protocols}/query_parameter.py (100%) diff --git a/metricflow/engine/metricflow_engine.py b/metricflow/engine/metricflow_engine.py index a83c1e3feb..b2b9a757d4 100644 --- a/metricflow/engine/metricflow_engine.py +++ b/metricflow/engine/metricflow_engine.py @@ -52,7 +52,7 @@ from metricflow.query.query_parser import MetricFlowQueryParser from metricflow.random_id import random_id from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_parameter import QueryParameterDimension, QueryParameterMetric +from metricflow.protocols.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.specs.specs import InstanceSpecSet, MetricFlowQuerySpec from metricflow.sql.optimizer.optimization_levels import SqlQueryOptimizationLevel from metricflow.telemetry.models import TelemetryLevel diff --git a/metricflow/specs/query_interface.py b/metricflow/protocols/query_interface.py similarity index 100% rename from metricflow/specs/query_interface.py rename to metricflow/protocols/query_interface.py diff --git a/metricflow/specs/query_parameter.py b/metricflow/protocols/query_parameter.py similarity index 100% rename from metricflow/specs/query_parameter.py rename to metricflow/protocols/query_parameter.py diff --git a/metricflow/query/query_parser.py b/metricflow/query/query_parser.py index 18cd8e9dea..397605fe6c 100644 --- a/metricflow/query/query_parser.py +++ b/metricflow/query/query_parser.py @@ -30,7 +30,7 @@ from metricflow.naming.linkable_spec_name import StructuredLinkableSpecName from metricflow.query.query_exceptions import InvalidQueryException from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_parameter import QueryParameterDimension, QueryParameterMetric +from metricflow.protocols.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.specs.specs import ( DimensionSpec, EntitySpec, diff --git a/metricflow/specs/where_filter_dimension.py b/metricflow/specs/where_filter_dimension.py index 14404ea7e1..4184bc98eb 100644 --- a/metricflow/specs/where_filter_dimension.py +++ b/metricflow/specs/where_filter_dimension.py @@ -16,7 +16,7 @@ from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import ( +from metricflow.protocols.query_interface import ( QueryInterfaceDimension, QueryInterfaceDimensionFactory, ) diff --git a/metricflow/specs/where_filter_entity.py b/metricflow/specs/where_filter_entity.py index f935384493..7d7493c263 100644 --- a/metricflow/specs/where_filter_entity.py +++ b/metricflow/specs/where_filter_entity.py @@ -13,7 +13,7 @@ from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import QueryInterfaceEntity, QueryInterfaceEntityFactory +from metricflow.protocols.query_interface import QueryInterfaceEntity, QueryInterfaceEntityFactory from metricflow.specs.specs import EntitySpec diff --git a/metricflow/specs/where_filter_time_dimension.py b/metricflow/specs/where_filter_time_dimension.py index 7436dddcb1..c6bb64a466 100644 --- a/metricflow/specs/where_filter_time_dimension.py +++ b/metricflow/specs/where_filter_time_dimension.py @@ -11,7 +11,7 @@ from metricflow.errors.errors import InvalidQuerySyntax from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.specs.query_interface import QueryInterfaceTimeDimension, QueryInterfaceTimeDimensionFactory +from metricflow.protocols.query_interface import QueryInterfaceTimeDimension, QueryInterfaceTimeDimensionFactory from metricflow.specs.specs import TimeDimensionSpec From 3cc443d1634d909043524abdda87e1f2940690c7 Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Wed, 20 Sep 2023 11:32:19 -0500 Subject: [PATCH 6/7] lint & format --- metricflow/engine/metricflow_engine.py | 2 +- metricflow/protocols/query_parameter.py | 1 + metricflow/query/query_parser.py | 4 ++-- metricflow/specs/where_filter_dimension.py | 2 +- metricflow/specs/where_filter_entity.py | 2 +- metricflow/specs/where_filter_time_dimension.py | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/metricflow/engine/metricflow_engine.py b/metricflow/engine/metricflow_engine.py index b2b9a757d4..21f7bc899b 100644 --- a/metricflow/engine/metricflow_engine.py +++ b/metricflow/engine/metricflow_engine.py @@ -47,12 +47,12 @@ DataflowToExecutionPlanConverter, ) from metricflow.plan_conversion.dataflow_to_sql import DataflowToSqlQueryPlanConverter +from metricflow.protocols.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.protocols.sql_client import SqlClient from metricflow.query.query_exceptions import InvalidQueryException from metricflow.query.query_parser import MetricFlowQueryParser from metricflow.random_id import random_id from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.protocols.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.specs.specs import InstanceSpecSet, MetricFlowQuerySpec from metricflow.sql.optimizer.optimization_levels import SqlQueryOptimizationLevel from metricflow.telemetry.models import TelemetryLevel diff --git a/metricflow/protocols/query_parameter.py b/metricflow/protocols/query_parameter.py index 0f681e64f5..2b6b4e02bc 100644 --- a/metricflow/protocols/query_parameter.py +++ b/metricflow/protocols/query_parameter.py @@ -3,6 +3,7 @@ from typing import Optional, Protocol from dbt_semantic_interfaces.type_enums import TimeGranularity + from metricflow.time.date_part import DatePart diff --git a/metricflow/query/query_parser.py b/metricflow/query/query_parser.py index 397605fe6c..a8e7deda0b 100644 --- a/metricflow/query/query_parser.py +++ b/metricflow/query/query_parser.py @@ -28,9 +28,9 @@ from metricflow.filters.time_constraint import TimeRangeConstraint from metricflow.model.semantic_manifest_lookup import SemanticManifestLookup from metricflow.naming.linkable_spec_name import StructuredLinkableSpecName +from metricflow.protocols.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.query.query_exceptions import InvalidQueryException from metricflow.specs.column_assoc import ColumnAssociationResolver -from metricflow.protocols.query_parameter import QueryParameterDimension, QueryParameterMetric from metricflow.specs.specs import ( DimensionSpec, EntitySpec, @@ -667,7 +667,7 @@ def _parse_linkable_elements( self, metric_references: Sequence[MetricReference], qualified_linkable_names: Optional[Sequence[str]] = None, - linkable_elements: Optional[Sequence[QueryParameter]] = None, + linkable_elements: Optional[Sequence[QueryParameterDimension]] = None, ) -> QueryTimeLinkableSpecSet: """Convert the linkable spec names into the respective specification objects.""" # TODO: refactor to only support group_by object inputs (removing group_by_names param) diff --git a/metricflow/specs/where_filter_dimension.py b/metricflow/specs/where_filter_dimension.py index 4184bc98eb..655fac2053 100644 --- a/metricflow/specs/where_filter_dimension.py +++ b/metricflow/specs/where_filter_dimension.py @@ -15,11 +15,11 @@ from typing_extensions import override from metricflow.errors.errors import InvalidQuerySyntax -from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.protocols.query_interface import ( QueryInterfaceDimension, QueryInterfaceDimensionFactory, ) +from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.specs import DimensionSpec diff --git a/metricflow/specs/where_filter_entity.py b/metricflow/specs/where_filter_entity.py index 7d7493c263..78af16c458 100644 --- a/metricflow/specs/where_filter_entity.py +++ b/metricflow/specs/where_filter_entity.py @@ -12,8 +12,8 @@ from typing_extensions import override from metricflow.errors.errors import InvalidQuerySyntax -from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.protocols.query_interface import QueryInterfaceEntity, QueryInterfaceEntityFactory +from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.specs import EntitySpec diff --git a/metricflow/specs/where_filter_time_dimension.py b/metricflow/specs/where_filter_time_dimension.py index c6bb64a466..6dd2bf7388 100644 --- a/metricflow/specs/where_filter_time_dimension.py +++ b/metricflow/specs/where_filter_time_dimension.py @@ -10,8 +10,8 @@ from typing_extensions import override from metricflow.errors.errors import InvalidQuerySyntax -from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.protocols.query_interface import QueryInterfaceTimeDimension, QueryInterfaceTimeDimensionFactory +from metricflow.specs.column_assoc import ColumnAssociationResolver from metricflow.specs.specs import TimeDimensionSpec From 3068b69f4825995ebd18c51fe8a8c46a9c37182a Mon Sep 17 00:00:00 2001 From: Devon Fulcher Date: Wed, 20 Sep 2023 11:42:48 -0500 Subject: [PATCH 7/7] fixed typing --- .../specs/query_param_implementations.py | 1 + metricflow/test/conftest.py | 3 ++- metricflow/test/query/test_query_parser.py | 22 ++++++++++--------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/metricflow/specs/query_param_implementations.py b/metricflow/specs/query_param_implementations.py index 62c42d6937..b791c46189 100644 --- a/metricflow/specs/query_param_implementations.py +++ b/metricflow/specs/query_param_implementations.py @@ -15,6 +15,7 @@ class DimensionQueryParameter: name: str grain: Optional[TimeGranularity] = None + descending: bool = False date_part: Optional[DatePart] = None def __post_init__(self) -> None: # noqa: D diff --git a/metricflow/test/conftest.py b/metricflow/test/conftest.py index 13318c9f16..d2a60d713c 100644 --- a/metricflow/test/conftest.py +++ b/metricflow/test/conftest.py @@ -18,9 +18,10 @@ @dataclass -class MockQueryParameter: +class MockQueryParameterDimension: """This is a mock that is just used to test the query parser.""" name: str grain: Optional[TimeGranularity] = None + descending: bool = False date_part: Optional[DatePart] = None diff --git a/metricflow/test/query/test_query_parser.py b/metricflow/test/query/test_query_parser.py index f8469d645c..e861369302 100644 --- a/metricflow/test/query/test_query_parser.py +++ b/metricflow/test/query/test_query_parser.py @@ -21,7 +21,7 @@ OrderBySpec, TimeDimensionSpec, ) -from metricflow.test.conftest import MockQueryParameter +from metricflow.test.conftest import MockQueryParameterDimension 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 @@ -189,11 +189,11 @@ def test_query_parser_with_object_params(bookings_query_parser: MetricFlowQueryP Metric = namedtuple("Metric", ["name", "descending"]) metric = Metric("bookings", False) group_by = [ - MockQueryParameter("booking__is_instant"), - MockQueryParameter("listing"), - MockQueryParameter(MTD), + MockQueryParameterDimension("booking__is_instant"), + MockQueryParameterDimension("listing"), + MockQueryParameterDimension(MTD), ] - order_by = [MockQueryParameter(MTD), MockQueryParameter("-bookings")] + order_by = [MockQueryParameterDimension(MTD), MockQueryParameterDimension("-bookings")] 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 == ( @@ -414,32 +414,34 @@ def test_date_part_parsing() -> None: with pytest.raises(RequestTimeGranularityException): query_parser.parse_and_validate_query( metric_names=["revenue"], - group_by=[MockQueryParameter(name="metric_time", date_part=DatePart.DOW)], + group_by=[MockQueryParameterDimension(name="metric_time", date_part=DatePart.DOW)], ) # Can't query date part for cumulative metrics with pytest.raises(UnableToSatisfyQueryError): query_parser.parse_and_validate_query( metric_names=["revenue_cumulative"], - group_by=[MockQueryParameter(name="metric_time", date_part=DatePart.YEAR)], + group_by=[MockQueryParameterDimension(name="metric_time", date_part=DatePart.YEAR)], ) # Can't query date part for metrics with offset to grain with pytest.raises(UnableToSatisfyQueryError): query_parser.parse_and_validate_query( metric_names=["revenue_since_start_of_year"], - group_by=[MockQueryParameter(name="metric_time", date_part=DatePart.MONTH)], + group_by=[MockQueryParameterDimension(name="metric_time", date_part=DatePart.MONTH)], ) # Requested granularity doesn't match resolved granularity with pytest.raises(RequestTimeGranularityException): query_parser.parse_and_validate_query( metric_names=["revenue"], - group_by=[MockQueryParameter(name="metric_time", grain=TimeGranularity.YEAR, date_part=DatePart.MONTH)], + group_by=[ + MockQueryParameterDimension(name="metric_time", grain=TimeGranularity.YEAR, date_part=DatePart.MONTH) + ], ) # Date part is compatible query_parser.parse_and_validate_query( metric_names=["revenue"], - group_by=[MockQueryParameter(name="metric_time", date_part=DatePart.MONTH)], + group_by=[MockQueryParameterDimension(name="metric_time", date_part=DatePart.MONTH)], )