Skip to content

Commit

Permalink
[CT-2696] Skip jinia parsing of metric filters (#7885)
Browse files Browse the repository at this point in the history
* Update metric filters in testing fixtures

I incorrectly wrote the tests such that they didn't include curly
braces, `{{..}}`, around things like `dimension(..)` for filters.
This updates the tests fixtures to have proper filter specifications

* Skip jinja rendering of `filter` key of metrics

Note that `filter` can show up in multiple places: as a root key
on a metric (`metric.filter`), on a metric input (`metric.type_params.metrics[x].filter`),
denominator (`metric.type_params.denominator.filter`), numerator
(`metric.type_params.numerator.filter`), and a metric input measure
(`metric.type_params.measure.filter` and `metric.type_params.measures[x].filter`).
In this commit we skip all of them :)

* Add changie doc for skipping jinja parsing for metric filters

* Update yaml renderer test for metrics
  • Loading branch information
QMalcolm authored Jun 15, 2023
1 parent 39e0c22 commit fafab5d
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 19 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20230615-142949.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Skip jinja parsing of metric filters
time: 2023-06-15T14:29:49.900201-07:00
custom:
Author: QMalcolm
Issue: "7864"
4 changes: 2 additions & 2 deletions core/dbt/parser/schema_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ def should_render_keypath(self, keypath: Keypath) -> bool:
elif self._is_norender_key(keypath[0:]):
return False
elif self.key == "metrics":
# back compat: "expression" is new name, "sql" is old name
if keypath[0] in ("expression", "sql"):
# This ensures all key paths that end in 'filter' for a metric are skipped
if keypath[-1] == "filter":
return False
elif self._is_norender_key(keypath[0:]):
return False
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/duplicates/test_duplicate_metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
type_params:
measure:
name: "years_tenure"
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
"""


Expand Down
10 changes: 5 additions & 5 deletions tests/functional/metrics/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
type_params:
measure:
name: "years_tenure"
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
- name: average_tenure
label: "Average tenure"
Expand Down Expand Up @@ -86,7 +86,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
- name: collective_window
label: "Collective window"
Expand All @@ -95,7 +95,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
window: 14 days
"""
Expand Down Expand Up @@ -377,7 +377,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
"""

Expand All @@ -404,7 +404,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
"""

Expand Down
8 changes: 4 additions & 4 deletions tests/functional/partial_parsing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@
type_params:
measure:
name: customers
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
+meta:
is_okr: True
tags:
Expand Down Expand Up @@ -441,7 +441,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
"""

Expand Down Expand Up @@ -588,7 +588,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
"""

Expand Down Expand Up @@ -977,7 +977,7 @@
type_params:
measure:
name: years_tenure
filter: "loves_dbt is true"
filter: "{{dimension('loves_dbt')}} is true"
"""

Expand Down
12 changes: 5 additions & 7 deletions tests/unit/test_yaml_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,20 @@ def test__macros(self):
self.assertEqual(dct, expected)

def test__metrics(self):
context = {"my_time_grains": "[day]"}
context = {"metric_name_end": "_metric"}
renderer = SchemaYamlRenderer(context, "metrics")

dct = {
"name": "my_source",
"name": "test{{ metric_name_end }}",
"description": "{{ docs('my_doc') }}",
"expression": "select {{ var('my_var') }} from my_table",
"time_grains": "{{ my_time_grains }}",
"filter": "{{ dimension('my_dim') }} = false",
}
# We expect the expression and description will not be rendered, but
# other fields will be
expected = {
"name": "my_source",
"name": "test_metric",
"description": "{{ docs('my_doc') }}",
"expression": "select {{ var('my_var') }} from my_table",
"time_grains": "[day]",
"filter": "{{ dimension('my_dim') }} = false",
}
dct = renderer.render_data(dct)
self.assertEqual(dct, expected)

0 comments on commit fafab5d

Please sign in to comment.