Skip to content

Commit

Permalink
Re-categorize TypeErrors that arise from create_from_where_filter
Browse files Browse the repository at this point in the history
… into `InvalidQueryException` (#817)

Resolves https://dbt-labs.slack.com/archives/C04TRSFQN75/p1697568904350429 (internal to dbt Labs)

Description
The intent of this PR is to re-categorize TypeErrors that arise from create_from_where_filter into InvalidQueryException. That way, these errors can be caught and handled properly. Additionally, I move the try-catch block for ParseWhereFilterException to be within the body of create_from_where_er that way all call sites can benefit from that catch.
  • Loading branch information
DevonFulcher authored Oct 25, 2023
1 parent e7ccd73 commit 9cd2d09
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 39 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20231017-155210.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: re-categorize `TypeErrors` that arise from `create_from_where_filter` into `InvalidQueryException`
time: 2023-10-17T15:52:10.948956-05:00
custom:
Author: DevonFulcher
Issue: None
12 changes: 3 additions & 9 deletions metricflow/query/query_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from dataclasses import dataclass
from typing import Dict, List, Optional, Sequence, Tuple

from dbt_semantic_interfaces.call_parameter_sets import ParseWhereFilterException
from dbt_semantic_interfaces.implementations.filters.where_filter import (
PydanticWhereFilter,
PydanticWhereFilterIntersection,
Expand Down Expand Up @@ -445,14 +444,9 @@ def _parse_and_validate_query(
)
where_filter_spec: Optional[WhereFilterSpec] = None
if where_filter is not None:
try:
where_filter_spec = WhereSpecFactory(
column_association_resolver=self._column_association_resolver,
).create_from_where_filter(where_filter)
except ParseWhereFilterException as e:
raise InvalidQueryException(
f"Error parsing the where filter: {where_filter.where_sql_template}. {e}"
) from e
where_filter_spec = WhereSpecFactory(
column_association_resolver=self._column_association_resolver,
).create_from_where_filter(where_filter)

where_spec_set = QueryTimeLinkableSpecSet.create_from_linkable_spec_set(where_filter_spec.linkable_spec_set)
requested_linkable_specs_with_requested_filter_specs = QueryTimeLinkableSpecSet.combine(
Expand Down
78 changes: 48 additions & 30 deletions metricflow/specs/where_filter_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
from typing import Optional

import jinja2
from dbt_semantic_interfaces.call_parameter_sets import ParseWhereFilterException
from dbt_semantic_interfaces.protocols import WhereFilterIntersection
from dbt_semantic_interfaces.protocols.where_filter import WhereFilter

from metricflow.filters.merge_where import merge_to_single_where_filter
from metricflow.query.query_exceptions import InvalidQueryException
from metricflow.specs.column_assoc import ColumnAssociationResolver
from metricflow.specs.specs import LinkableSpecSet, WhereFilterSpec
from metricflow.specs.where_filter_dimension import WhereFilterDimensionFactory
Expand Down Expand Up @@ -45,17 +47,15 @@ def create_from_where_filter_intersection( # noqa: D

return self.create_from_where_filter(where_filter)

def create_from_where_filter(self, where_filter: WhereFilter) -> WhereFilterSpec: # noqa: D
# Used to check that call parameter sets generated by DSI match those generated below.
call_parameter_sets = where_filter.call_parameter_sets

dimension_factory = WhereFilterDimensionFactory(call_parameter_sets, self._column_association_resolver)
time_dimension_factory = WhereFilterTimeDimensionFactory(call_parameter_sets, self._column_association_resolver)
entity_factory = WhereFilterEntityFactory(call_parameter_sets, self._column_association_resolver)
def _render_sql_template(
self,
where_filter: WhereFilter,
dimension_factory: WhereFilterDimensionFactory,
time_dimension_factory: WhereFilterTimeDimensionFactory,
entity_factory: WhereFilterEntityFactory,
) -> str:
try:
rendered_sql_template = jinja2.Template(
where_filter.where_sql_template, undefined=jinja2.StrictUndefined
).render(
return jinja2.Template(where_filter.where_sql_template, undefined=jinja2.StrictUndefined).render(
{
"Dimension": dimension_factory.create,
"TimeDimension": time_dimension_factory.create,
Expand All @@ -67,23 +67,41 @@ def create_from_where_filter(self, where_filter: WhereFilter) -> WhereFilterSpec
f"Error while rendering Jinja template:\n{where_filter.where_sql_template}"
) from e

"""
Dimensions that are created with a grain parameter, Dimension(...).grain(...), are
added to dimension_specs otherwise they are add to time_dimension_factory.time_dimension_specs
"""
dimension_specs = []
for dimension in dimension_factory.created:
if dimension.time_dimension_spec:
time_dimension_factory.time_dimension_specs.append(dimension.time_dimension_spec)
else:
dimension_specs.append(dimension.dimension_spec)

return WhereFilterSpec(
where_sql=rendered_sql_template,
bind_parameters=self._bind_parameters,
linkable_spec_set=LinkableSpecSet(
dimension_specs=tuple(dimension_specs),
time_dimension_specs=tuple(time_dimension_factory.time_dimension_specs),
entity_specs=tuple(entity_factory.entity_specs),
),
)
def create_from_where_filter(self, where_filter: WhereFilter) -> WhereFilterSpec:
"""Generates WhereFilterSpec using Jinja."""
try:
call_parameter_sets = where_filter.call_parameter_sets

dimension_factory = WhereFilterDimensionFactory(call_parameter_sets, self._column_association_resolver)
time_dimension_factory = WhereFilterTimeDimensionFactory(
call_parameter_sets, self._column_association_resolver
)
entity_factory = WhereFilterEntityFactory(call_parameter_sets, self._column_association_resolver)
rendered_sql_template = self._render_sql_template(
where_filter, dimension_factory, time_dimension_factory, entity_factory
)

"""
Dimensions that are created with a grain parameter, Dimension(...).grain(...), are
added to dimension_specs otherwise they are add to time_dimension_factory.time_dimension_specs
"""
dimension_specs = []
for dimension in dimension_factory.created:
if dimension.time_dimension_spec:
time_dimension_factory.time_dimension_specs.append(dimension.time_dimension_spec)
else:
dimension_specs.append(dimension.dimension_spec)

return WhereFilterSpec(
where_sql=rendered_sql_template,
bind_parameters=self._bind_parameters,
linkable_spec_set=LinkableSpecSet(
dimension_specs=tuple(dimension_specs),
time_dimension_specs=tuple(time_dimension_factory.time_dimension_specs),
entity_specs=tuple(entity_factory.entity_specs),
),
)
except (ParseWhereFilterException, TypeError) as e:
raise InvalidQueryException(
f"Error parsing the where filter: {where_filter.where_sql_template}. {e}"
) from e
11 changes: 11 additions & 0 deletions metricflow/test/model/test_where_filter_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

import logging

import pytest
from dbt_semantic_interfaces.implementations.filters.where_filter import PydanticWhereFilter
from dbt_semantic_interfaces.references import EntityReference
from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity

from metricflow.query.query_exceptions import InvalidQueryException
from metricflow.specs.column_assoc import ColumnAssociationResolver
from metricflow.specs.specs import (
DimensionSpec,
Expand Down Expand Up @@ -62,6 +64,15 @@ def test_dimension_in_filter_with_grain( # noqa: D
)


def test_time_dimension_without_grain(column_association_resolver: ColumnAssociationResolver) -> None: # noqa
where_filter = PydanticWhereFilter(where_sql_template="{{ TimeDimension('metric_time') }} > '2023-10-17'")

with pytest.raises(InvalidQueryException):
WhereSpecFactory(
column_association_resolver=column_association_resolver,
).create_from_where_filter(where_filter)


def test_time_dimension_in_filter( # noqa: D
column_association_resolver: ColumnAssociationResolver,
) -> None:
Expand Down

0 comments on commit 9cd2d09

Please sign in to comment.