Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro committed Sep 4, 2024
1 parent cdbe773 commit bb73f1a
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 33 deletions.
25 changes: 14 additions & 11 deletions docs/docs/configuration/sql-templating.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -348,35 +348,38 @@ Here's a concrete example:
**Time Filter for a Specific Column**
The `{{ get_time_filter() }}` macro returns the time filter applied to a specific column. The macro takes the following
parameters:
The `{{ get_time_filter() }}` macro returns the time filter applied to a specific column. This is useful if you want
to handle time filters inside the virtual dataset, as by default the time filter is placed on the outer query. This can
have considerable performance implications, as many databases and query engines are able to optimize the query better
if the temporal filter is placed on the inner query, as opposed to the outer query.
The macro takes the following parameters:
- `column`: Name of the temporal column. Leave undefined to reference the time range from a Dashboard Native Time Range
filter (when present).
- `target_type`: The target temporal type as recognized by the target database (e.g. `TIMESTAMP`, `DATE` or
`DATETIME`). If `column` is defined, the format will default to the type of the column. This is used to produce
the format of the `from_dttm` and `to_dttm` properties of the returned `TimeFilter` object. Note, that omitting
`column` and `target_type` will render format the temporal values as ISO format.
the format of the `from_expr` and `to_expr` properties of the returned `TimeFilter` object.
The return type has the following properties:
- `since`: the start of the time filter (if any)
- `until`: the end of the time filter (if any)
- `from_expr`: the start of the time filter (if any)
- `to_expr`: the end of the time filter (if any)
- `time_range`: The time range as defined on the
Here's a concrete example using the `logs` table from the Superset metastore:
```
{% set time_filter = get_time_filter("dttm", remove_filter=True) %}
{% set since = time_filter.since %}
{% set until = time_filter.until %}
{% set from_expr = time_filter.from_expr %}
{% set to_expr = time_filter.to_expr %}
{% set time_range = time_filter.time_range %}

SELECT
*,
{% if time_range %}'{{ time_range }}'{% else %}''{% endif %} as time_range
FROM logs
{% if since or until %}WHERE 1 = 1
{% if since %}AND dttm >= {{ since }}{% endif %}
{% if until %}AND dttm < {{ until }}{% endif %}
{% if from_expr or to_expr %}WHERE 1 = 1
{% if from_expr %}AND dttm >= {{ from_expr }}{% endif %}
{% if to_expr %}AND dttm < {{ to_expr }}{% endif %}
{% endif %}
```
Expand Down
22 changes: 11 additions & 11 deletions superset/jinja_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ class TimeFilter:
Container for temporal filter.
"""

since: str | None
until: str | None
from_expr: str | None
to_expr: str | None
time_range: str | None


Expand Down Expand Up @@ -394,15 +394,15 @@ def get_time_filter(
Usage example::
{% set time_filter = get_time_filter("dttm", remove_filter=True) %}
{% set since = time_filter.since %}
{% set until = time_filter.until %}
{% set from_expr = time_filter.from_expr %}
{% set to_expr = time_filter.to_expr %}
{% set time_range = time_filter.time_range %}
select *,
{% if time_range %}'{{ time_range }}'{% else %}''{% endif %} as time_range
from logs
{% if since or until %}where 1 = 1
{% if since %}and dttm >= {{ since }}{% endif %}
{% if until %}and dttm < {{ until }}{% endif %}
{% if from_expr or to_expr %}where 1 = 1
{% if from_expr %}and dttm >= {{ from_expr }}{% endif %}
{% if to_expr %}and dttm < {{ to_expr }}{% endif %}
{% endif %}
This will render the time filter inside the virtual dataset subquery with the
Expand All @@ -413,7 +413,7 @@ def get_time_filter(
:param target_type: The target temporal type as recognized by the target
database (e.g. `TIMESTAMP`, `DATE` or `DATETIME`). If `column` is defined,
the format will default to the type of the column. This is used to produce
the format of the `since` and `until` properties of the returned
the format of the `from_expr` and `to_expr` properties of the returned
`TimeFilter` object. Note, that omitting `column` and `target_type` will
render format the temporal values as ISO format.
:param remove_filter: When set to true, mark the filter as processed,
Expand Down Expand Up @@ -450,7 +450,7 @@ def get_time_filter(
target_type = self.table.columns_types.get(column)

Check warning on line 450 in superset/jinja_context.py

View check run for this annotation

Codecov / codecov/patch

superset/jinja_context.py#L448-L450

Added lines #L448 - L450 were not covered by tests

time_range = time_range or NO_TIME_RANGE
since, until = get_since_until_from_time_range(time_range)
from_expr, to_expr = get_since_until_from_time_range(time_range)

Check warning on line 453 in superset/jinja_context.py

View check run for this annotation

Codecov / codecov/patch

superset/jinja_context.py#L452-L453

Added lines #L452 - L453 were not covered by tests

def _format_dttm(dttm: datetime | None) -> str | None:
if target_type is None and dttm:
Expand All @@ -462,8 +462,8 @@ def _format_dttm(dttm: datetime | None) -> str | None:
)

return TimeFilter(

Check warning on line 464 in superset/jinja_context.py

View check run for this annotation

Codecov / codecov/patch

superset/jinja_context.py#L464

Added line #L464 was not covered by tests
since=_format_dttm(since),
until=_format_dttm(until),
from_expr=_format_dttm(from_expr),
to_expr=_format_dttm(to_expr),
time_range=time_range,
)

Expand Down
22 changes: 11 additions & 11 deletions tests/unit_tests/jinja_context_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,8 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
"postgresql://",
[{}],
TimeFilter(
since=None,
until=None,
from_expr=None,
to_expr=None,
time_range="No filter",
),
[],
Expand All @@ -862,8 +862,8 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
"postgresql://",
[{"time_range": "Last week"}],
TimeFilter(
since="TO_TIMESTAMP('2024-08-26 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
until="TO_TIMESTAMP('2024-09-02 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
from_expr="TO_TIMESTAMP('2024-08-26 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
to_expr="TO_TIMESTAMP('2024-09-02 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
time_range="Last week",
),
[],
Expand All @@ -885,8 +885,8 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
}
],
TimeFilter(
since="TO_TIMESTAMP('2024-08-26 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
until="TO_TIMESTAMP('2024-09-02 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
from_expr="TO_TIMESTAMP('2024-08-26 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
to_expr="TO_TIMESTAMP('2024-09-02 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
time_range="Last week",
),
[],
Expand All @@ -908,8 +908,8 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
}
],
TimeFilter(
since="TO_DATE('2024-08-26', 'YYYY-MM-DD')",
until="TO_DATE('2024-09-02', 'YYYY-MM-DD')",
from_expr="TO_DATE('2024-08-26', 'YYYY-MM-DD')",
to_expr="TO_DATE('2024-09-02', 'YYYY-MM-DD')",
time_range="Last week",
),
["dt"],
Expand All @@ -931,8 +931,8 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
}
],
TimeFilter(
since="DATE '2024-08-02'",
until="DATE '2024-09-02'",
from_expr="DATE '2024-08-02'",
to_expr="DATE '2024-09-02'",
time_range="Last month",
),
["dttm"],
Expand Down Expand Up @@ -966,7 +966,7 @@ def test_get_time_filter(
)

with (
freeze_time("2024-09-03"),
freeze_time("2024-09-03T00:00:00Z"),
app.test_request_context(
json={"queries": queries},
),
Expand Down

0 comments on commit bb73f1a

Please sign in to comment.