diff --git a/dbt_semantic_interfaces/parsing/text_input/description_renderer.py b/dbt_semantic_interfaces/parsing/text_input/description_renderer.py index 404404a9..3cbe7ac4 100644 --- a/dbt_semantic_interfaces/parsing/text_input/description_renderer.py +++ b/dbt_semantic_interfaces/parsing/text_input/description_renderer.py @@ -3,7 +3,7 @@ from abc import ABC, abstractmethod from dbt_semantic_interfaces.parsing.text_input.ti_description import ( - QueryItemDescription, + ObjectBuilderItemDescription, ) @@ -22,6 +22,6 @@ class QueryItemDescriptionRenderer(ABC): """ @abstractmethod - def render_description(self, item_description: QueryItemDescription) -> str: + def render_description(self, item_description: ObjectBuilderItemDescription) -> str: """Return the string that will be substituted for the query item in the Jinja template.""" raise NotImplementedError diff --git a/dbt_semantic_interfaces/parsing/text_input/rendering_helper.py b/dbt_semantic_interfaces/parsing/text_input/rendering_helper.py index d4e5aa8f..1ce9086a 100644 --- a/dbt_semantic_interfaces/parsing/text_input/rendering_helper.py +++ b/dbt_semantic_interfaces/parsing/text_input/rendering_helper.py @@ -6,8 +6,8 @@ from typing_extensions import override from dbt_semantic_interfaces.parsing.text_input.ti_description import ( + ObjectBuilderItemDescription, ObjectBuilderMethod, - QueryItemDescription, QueryItemType, ) from dbt_semantic_interfaces.parsing.text_input.ti_exceptions import ( @@ -16,7 +16,7 @@ if typing.TYPE_CHECKING: from dbt_semantic_interfaces.parsing.text_input.ti_processor import ( - QueryItemDescriptionProcessor, + ObjectBuilderItemDescriptionProcessor, ) from dbt_semantic_interfaces.parsing.text_input.valid_method import ValidMethodMapping @@ -40,7 +40,7 @@ class ObjectBuilderJinjaRenderHelper: def __init__( # noqa: D107 self, - description_processor: QueryItemDescriptionProcessor, + description_processor: ObjectBuilderItemDescriptionProcessor, valid_method_mapping: ValidMethodMapping, ) -> None: self._description_processor = description_processor @@ -56,7 +56,7 @@ def _create(name: str, entity_path: Sequence[str] = ()) -> _RenderingClassForJin return _RenderingClassForJinjaTemplate( description_processor=description_processor, allowed_methods=allowed_methods, - initial_item_description=QueryItemDescription( + initial_item_description=ObjectBuilderItemDescription( item_type=item_type, item_name=name, entity_path=tuple(entity_path), @@ -85,7 +85,7 @@ def _create( return _RenderingClassForJinjaTemplate( description_processor=description_processor, allowed_methods=allowed_methods, - initial_item_description=QueryItemDescription( + initial_item_description=ObjectBuilderItemDescription( item_type=item_type, item_name=time_dimension_name, entity_path=tuple(entity_path), @@ -108,7 +108,7 @@ def _create(entity_name: str, entity_path: Sequence[str] = ()) -> _RenderingClas return _RenderingClassForJinjaTemplate( description_processor=description_processor, allowed_methods=allowed_methods, - initial_item_description=QueryItemDescription( + initial_item_description=ObjectBuilderItemDescription( item_type=item_type, item_name=entity_name, entity_path=tuple(entity_path), @@ -131,7 +131,7 @@ def _create(metric_name: str, group_by: Sequence[str] = ()) -> _RenderingClassFo return _RenderingClassForJinjaTemplate( description_processor=description_processor, allowed_methods=allowed_methods, - initial_item_description=QueryItemDescription( + initial_item_description=ObjectBuilderItemDescription( item_type=item_type, item_name=metric_name, entity_path=(), @@ -159,9 +159,9 @@ class _RenderingClassForJinjaTemplate: def __init__( self, - description_processor: QueryItemDescriptionProcessor, + description_processor: ObjectBuilderItemDescriptionProcessor, allowed_methods: FrozenSet[ObjectBuilderMethod], - initial_item_description: QueryItemDescription, + initial_item_description: ObjectBuilderItemDescription, ) -> None: """Initializer. @@ -178,7 +178,7 @@ def __init__( def _update_current_description( self, builder_method: ObjectBuilderMethod, - new_description: QueryItemDescription, + new_description: ObjectBuilderItemDescription, ) -> None: if builder_method not in self._allowed_builder_methods: raise InvalidBuilderMethodException( diff --git a/dbt_semantic_interfaces/parsing/text_input/ti_description.py b/dbt_semantic_interfaces/parsing/text_input/ti_description.py index 2093a0cf..6477b4f1 100644 --- a/dbt_semantic_interfaces/parsing/text_input/ti_description.py +++ b/dbt_semantic_interfaces/parsing/text_input/ti_description.py @@ -4,9 +4,15 @@ from enum import Enum from typing import Optional, Tuple +from dbt_semantic_interfaces.enum_extension import assert_values_exhausted +from dbt_semantic_interfaces.errors import InvalidQuerySyntax +from dbt_semantic_interfaces.naming.dundered import StructuredDunderedName +from dbt_semantic_interfaces.type_enums import TimeGranularity +from dbt_semantic_interfaces.type_enums.date_part import DatePart + @dataclass(frozen=True) -class QueryItemDescription: +class ObjectBuilderItemDescription: """Describes a query item specified by the user. For example, the following specified in an order-by of a saved query: @@ -15,7 +21,7 @@ class QueryItemDescription: -> - QueryItemDescription( + ObjectBuilderItemDescription( item_type=GroupByItemType.DIMENSION, item_name="user__created_at", entity_path=['listing'], @@ -38,25 +44,66 @@ class QueryItemDescription: descending: Optional[bool] def __post_init__(self) -> None: # noqa: D105 - if self.item_type is QueryItemType.ENTITY or self.item_type is QueryItemType.METRIC: - assert ( - self.time_granularity_name is None - ), f"{self.time_granularity_name=} is not supported for {self.item_type=}" - assert self.date_part_name is None, f"{self.date_part_name=} is not supported for {self.item_type=}" - - if self.item_type is not QueryItemType.METRIC: - assert ( - not self.group_by_for_metric_item - ), f"{self.group_by_for_metric_item=} is not supported for {self.item_type=}" + item_type = self.item_type + + # Check that time granularity and date part are only specified for dimensions and time dimensions. + if item_type is QueryItemType.ENTITY or item_type is QueryItemType.METRIC: + if self.time_granularity_name is not None: + raise InvalidQuerySyntax(f"{self.time_granularity_name=} is not supported for {item_type=}") + if self.date_part_name is not None: + raise InvalidQuerySyntax(f"{self.date_part_name=} is not supported for {item_type=}") + elif item_type is QueryItemType.TIME_DIMENSION or item_type is QueryItemType.DIMENSION: + pass + else: + assert_values_exhausted(item_type) + + structured_item_name = StructuredDunderedName.parse_name(self.item_name) + + # Check that metrics do not have an entity prefix or entity path. + if item_type is QueryItemType.METRIC: + if len(self.entity_path) > 0: + raise InvalidQuerySyntax("The entity path should not be specified for a metric.") + if len(structured_item_name.entity_links) > 0: + raise InvalidQuerySyntax("The name of the metric should not have entity links.") + # Check that dimensions / time dimensions have a valid time granularity / date part. + elif item_type is QueryItemType.DIMENSION or item_type is QueryItemType.TIME_DIMENSION: + if self.time_granularity_name is not None: + valid_time_granularity_names = set(time_granularity.value for time_granularity in TimeGranularity) + if self.time_granularity_name.lower() not in valid_time_granularity_names: + raise InvalidQuerySyntax( + f"{self.time_granularity_name!r} is not a valid time granularity. Valid values are" + f" {valid_time_granularity_names}" + ) + + if self.date_part_name is not None: + valid_date_part_names = set(date_part.value for date_part in DatePart) + if self.date_part_name.lower() not in set(date_part.value for date_part in DatePart): + raise InvalidQuerySyntax( + f"{self.time_granularity_name!r} is not a valid time granularity. Valid values are" + f" {valid_date_part_names}" + ) + + # Check that non-metric items don't specify group_by_for_metric_item. + if item_type is QueryItemType.METRIC: + pass + elif ( + item_type is QueryItemType.DIMENSION + or item_type is QueryItemType.ENTITY + or item_type is QueryItemType.TIME_DIMENSION + ): + if len(self.group_by_for_metric_item) > 0: + raise InvalidQuerySyntax("A group-by should only be specified for metrics.") + else: + assert_values_exhausted(item_type) def create_modified( self, time_granularity_name: Optional[str] = None, date_part_name: Optional[str] = None, descending: Optional[bool] = None, - ) -> QueryItemDescription: + ) -> ObjectBuilderItemDescription: """Create one with the same fields as self except the ones provided.""" - return QueryItemDescription( + return ObjectBuilderItemDescription( item_type=self.item_type, item_name=self.item_name, entity_path=self.entity_path, @@ -66,9 +113,9 @@ def create_modified( descending=descending or self.descending, ) - def with_descending_unset(self) -> QueryItemDescription: + def with_descending_unset(self) -> ObjectBuilderItemDescription: """Return this with the `descending` field set to None.""" - return QueryItemDescription( + return ObjectBuilderItemDescription( item_type=self.item_type, item_name=self.item_name, entity_path=self.entity_path, @@ -92,6 +139,12 @@ class QueryItemType(Enum): ENTITY = "Entity" METRIC = "Metric" + def __lt__(self, other) -> bool: # type: ignore[misc] + """Allow for ordering so that a sequence of these can be consistently represented for test snapshots.""" + if self.__class__ is other.__class__: + return self.value < other.value + return NotImplemented + class ObjectBuilderMethod(Enum): """In the object builder notation, the possible methods that can be called on the builder object. diff --git a/dbt_semantic_interfaces/parsing/text_input/ti_processor.py b/dbt_semantic_interfaces/parsing/text_input/ti_processor.py index f0ad2e8d..6823a6a4 100644 --- a/dbt_semantic_interfaces/parsing/text_input/ti_processor.py +++ b/dbt_semantic_interfaces/parsing/text_input/ti_processor.py @@ -17,7 +17,7 @@ ObjectBuilderJinjaRenderHelper, ) from dbt_semantic_interfaces.parsing.text_input.ti_description import ( - QueryItemDescription, + ObjectBuilderItemDescription, ) from dbt_semantic_interfaces.parsing.text_input.ti_exceptions import ( QueryItemJinjaException, @@ -25,16 +25,21 @@ from dbt_semantic_interfaces.parsing.text_input.valid_method import ValidMethodMapping -class QueryItemTextProcessor: +class ObjectBuilderTextProcessor: """Performs processing actions for text containing query items specified in the object-builder syntax. This currently supports: - * Collecting `QueryItemDescription`s from a Jinja template. + * Collecting `ObjectBuilderItemDescription`s from a Jinja template. * Rendering a Jinja template using a specified renderer. """ - def get_description(self, query_item_input: str, valid_method_mapping: ValidMethodMapping) -> QueryItemDescription: - """Get the `QueryItemDescription` for a single item e.g. `Dimension('listing__country').descending(True)`.""" + def get_description( + self, query_item_input: str, valid_method_mapping: ValidMethodMapping + ) -> ObjectBuilderItemDescription: + """Get the `ObjectBuilderItemDescription` for a single item. + + e.g. `Dimension('listing__country').descending(True)`. + """ descriptions = self.collect_descriptions_from_template( jinja_template="{{ " + query_item_input + " }}", valid_method_mapping=valid_method_mapping, @@ -49,8 +54,8 @@ def collect_descriptions_from_template( self, jinja_template: str, valid_method_mapping: ValidMethodMapping, - ) -> Sequence[QueryItemDescription]: - """Returns the `QueryItemDescription`s that are found in a Jinja template. + ) -> Sequence[ObjectBuilderItemDescription]: + """Returns the `ObjectBuilderItemDescription`s that are found in a Jinja template. Args: jinja_template: A Jinja-template string like `{{ Dimension('listing__country') }} = 'US'`. @@ -104,9 +109,9 @@ def _process_template( self, jinja_template: str, valid_method_mapping: ValidMethodMapping, - description_processor: QueryItemDescriptionProcessor, + description_processor: ObjectBuilderItemDescriptionProcessor, ) -> str: - """Helper to run a `QueryItemDescriptionProcessor` on a Jinja template.""" + """Helper to run a `ObjectBuilderItemDescriptionProcessor` on a Jinja template.""" render_helper = ObjectBuilderJinjaRenderHelper( description_processor=description_processor, valid_method_mapping=valid_method_mapping, @@ -131,34 +136,34 @@ def _process_template( return rendered -class QueryItemDescriptionProcessor(ABC): +class ObjectBuilderItemDescriptionProcessor(ABC): """General processor that does something to a query-item description seen in a Jinja template.""" @abstractmethod - def process_description(self, item_description: QueryItemDescription) -> str: + def process_description(self, item_description: ObjectBuilderItemDescription) -> str: """Process the given description, and return a string that would be substituted into the Jinja template.""" raise NotImplementedError -class _CollectDescriptionProcessor(QueryItemDescriptionProcessor): +class _CollectDescriptionProcessor(ObjectBuilderItemDescriptionProcessor): """Processor that collects all descriptions that were processed.""" def __init__(self) -> None: # noqa: D107 - self._items: List[QueryItemDescription] = [] + self._items: List[ObjectBuilderItemDescription] = [] - def collected_descriptions(self) -> Sequence[QueryItemDescription]: + def collected_descriptions(self) -> Sequence[ObjectBuilderItemDescription]: """Return all descriptions that were processed so far.""" return self._items @override - def process_description(self, item_description: QueryItemDescription) -> str: + def process_description(self, item_description: ObjectBuilderItemDescription) -> str: if item_description not in self._items: self._items.append(item_description) return "" -class _RendererProcessor(QueryItemDescriptionProcessor): +class _RendererProcessor(ObjectBuilderItemDescriptionProcessor): """Processor that renders the descriptions in a Jinja template using the given renderer. This is just a pass-through, but it allows `QueryItemDescriptionRenderer` to be a facade that has more appropriate @@ -169,5 +174,5 @@ def __init__(self, renderer: QueryItemDescriptionRenderer) -> None: # noqa: D10 self._renderer = renderer @override - def process_description(self, item_description: QueryItemDescription) -> str: + def process_description(self, item_description: ObjectBuilderItemDescription) -> str: return self._renderer.render_description(item_description) diff --git a/dbt_semantic_interfaces/parsing/where_filter/where_filter_parser.py b/dbt_semantic_interfaces/parsing/where_filter/where_filter_parser.py index 9cc40ecb..7d070b84 100644 --- a/dbt_semantic_interfaces/parsing/where_filter/where_filter_parser.py +++ b/dbt_semantic_interfaces/parsing/where_filter/where_filter_parser.py @@ -1,16 +1,18 @@ from __future__ import annotations +from typing import Sequence + from dbt_semantic_interfaces.call_parameter_sets import ( FilterCallParameterSets, ParseWhereFilterException, ) from dbt_semantic_interfaces.enum_extension import assert_values_exhausted -from dbt_semantic_interfaces.parsing.text_input.ti_description import QueryItemType -from dbt_semantic_interfaces.parsing.text_input.ti_exceptions import ( - QueryItemJinjaException, +from dbt_semantic_interfaces.parsing.text_input.ti_description import ( + ObjectBuilderItemDescription, + QueryItemType, ) from dbt_semantic_interfaces.parsing.text_input.ti_processor import ( - QueryItemTextProcessor, + ObjectBuilderTextProcessor, ) from dbt_semantic_interfaces.parsing.text_input.valid_method import ( ConfiguredValidMethodMapping, @@ -24,18 +26,23 @@ class WhereFilterParser: """Parses the template in the WhereFilter into FilterCallParameterSets.""" @staticmethod - def parse_call_parameter_sets(where_sql_template: str) -> FilterCallParameterSets: - """Return the result of extracting the semantic objects referenced in the where SQL template string.""" - text_processor = QueryItemTextProcessor() + def parse_item_descriptions(where_sql_template: str) -> Sequence[ObjectBuilderItemDescription]: + """Parses the filter and returns the item descriptions.""" + text_processor = ObjectBuilderTextProcessor() try: - descriptions = text_processor.collect_descriptions_from_template( + return text_processor.collect_descriptions_from_template( jinja_template=where_sql_template, valid_method_mapping=ConfiguredValidMethodMapping.DEFAULT_MAPPING, ) - except QueryItemJinjaException as e: + except Exception as e: raise ParseWhereFilterException(f"Error while parsing Jinja template:\n{where_sql_template}") from e + @staticmethod + def parse_call_parameter_sets(where_sql_template: str) -> FilterCallParameterSets: + """Return the result of extracting the semantic objects referenced in the where SQL template string.""" + descriptions = WhereFilterParser.parse_item_descriptions(where_sql_template) + """ Dimensions that are created with a grain or date_part parameter, for instance Dimension(...).grain(...), are added to time_dimension_call_parameter_sets otherwise they are add to dimension_call_parameter_sets diff --git a/dbt_semantic_interfaces/validations/saved_query.py b/dbt_semantic_interfaces/validations/saved_query.py index bc978341..7d3716bb 100644 --- a/dbt_semantic_interfaces/validations/saved_query.py +++ b/dbt_semantic_interfaces/validations/saved_query.py @@ -8,11 +8,11 @@ from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets from dbt_semantic_interfaces.naming.keywords import METRIC_TIME_ELEMENT_NAME from dbt_semantic_interfaces.parsing.text_input.ti_description import ( - QueryItemDescription, + ObjectBuilderItemDescription, QueryItemType, ) from dbt_semantic_interfaces.parsing.text_input.ti_processor import ( - QueryItemTextProcessor, + ObjectBuilderTextProcessor, ) from dbt_semantic_interfaces.parsing.text_input.valid_method import ( ConfiguredValidMethodMapping, @@ -142,7 +142,7 @@ def _check_where(saved_query: SavedQuery) -> Sequence[ValidationIssue]: @staticmethod def _parse_query_item( saved_query: SavedQuery, - text_processor: QueryItemTextProcessor, + text_processor: ObjectBuilderTextProcessor, query_item_input: str, element_type: SavedQueryElementType, valid_method_mapping: ValidMethodMapping, @@ -184,7 +184,7 @@ def _check_order_by(saved_query: SavedQuery) -> Sequence[ValidationIssue]: return validation_issues valid_query_item_descriptions = set() - text_processor = QueryItemTextProcessor() + text_processor = ObjectBuilderTextProcessor() for metric in saved_query.query_params.metrics: # In an order-by, a metric is specified as "Metric('bookings')" while in the metrics section, it's only the # metric name. @@ -299,7 +299,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati class _ParseQueryItemResult: """Result of parsing a string like `Dimension('listing__country')`.""" - item_description: Optional[QueryItemDescription] + item_description: Optional[ObjectBuilderItemDescription] validation_issue: Optional[ValidationIssue] def __post_init__(self) -> None: diff --git a/tests/parsing/test_object_builder_item_description.py b/tests/parsing/test_object_builder_item_description.py new file mode 100644 index 00000000..1b4abba2 --- /dev/null +++ b/tests/parsing/test_object_builder_item_description.py @@ -0,0 +1,49 @@ +import logging + +import pytest + +from dbt_semantic_interfaces.errors import InvalidQuerySyntax +from dbt_semantic_interfaces.parsing.text_input.ti_processor import ( + ObjectBuilderTextProcessor, +) +from dbt_semantic_interfaces.parsing.text_input.valid_method import ( + ConfiguredValidMethodMapping, +) + +logger = logging.getLogger(__name__) + + +def test_valid_object_builder_items() -> None: # noqa: D + text_processor = ObjectBuilderTextProcessor() + + valid_items = ( + "Dimension('listing__created_at', entity_path=['host'])", + "Dimension('listing__created_at', entity_path=['host']).grain('day').date_part('day')", + "TimeDimension('listing__created_at', time_granularity_name='day', entity_path=['host'], date_part_name='day')", + "Entity('listing__created_at', entity_path=['host'])", + "Metric('bookings', group_by=['listing__created_at'])", + ) + for valid_item in valid_items: + logger.info(f"Checking {valid_item=}") + text_processor.get_description(valid_item, ConfiguredValidMethodMapping.DEFAULT_MAPPING) + + +def test_invalid_object_builder_items() -> None: # noqa: D + text_processor = ObjectBuilderTextProcessor() + + invalid_items = ( + "Dimension('listing__created_at').grain('invalid')", + "Dimension('listing__created_at').date_part('invalid')", + "TimeDimension('listing__created_at', 'invalid', 'day')", + "TimeDimension('listing__created_at', 'day', date_part_name='invalid')", + "TimeDimension('listing__created_at', 'day', date_part_name='month').grain('month')", + "TimeDimension('listing__created_at', 'day', date_part_name='month').date_part('month')", + "Entity('listing__created_at').grain('day')", + "Entity('listing__created_at').date_part('day')", + "Metric('bookings').grain('day')", + "Metric('bookings').date_part('day')", + ) + for invalid_item in invalid_items: + with pytest.raises(InvalidQuerySyntax): + logger.info(f"Checking {invalid_item=}") + text_processor.get_description(invalid_item, ConfiguredValidMethodMapping.DEFAULT_MAPPING)