add default option

apache · Sep 5, 2024 · 288e6af · 288e6af
1 parent 00b2386
commit 288e6af
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 6 deletions.
diff --git a/docs/docs/configuration/sql-templating.mdx b/docs/docs/configuration/sql-templating.mdx
@@ -346,7 +346,7 @@ Here's a concrete example:
     order by lineage, level
 ```
 
-**Time Filter for a Specific Column**
+**Time Filter**
 
 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
@@ -356,9 +356,12 @@ if the temporal filter is placed on the inner query, as opposed to the outer que
 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).
+- `default`: The default value to fall back to if the time filter is not present, or has the value `No filter`
 - `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_expr` and `to_expr` properties of the returned `TimeFilter` object.
+- `remove_filter`: When set to true, mark the filter as processed, removing it from the outer query. Useful when a
+  filter should only apply to the inner query.
 
 The return type has the following properties:
 - `from_expr`: the start of the time filter (if any)
@@ -372,10 +375,9 @@ Here's a concrete example using the `logs` table from the Superset metastore:
 {% 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
+  '{{ time_range }}' as time_range
 FROM logs
 {% if from_expr or to_expr %}WHERE 1 = 1
 {% if from_expr %}AND dttm >= {{ from_expr }}{% endif %}
@@ -400,6 +402,20 @@ ORDER BY count DESC
 LIMIT 1000;
 ```
 
+When using the `default` parameter, the templated query can be simplified, as the endpoints will always be defined
+(to use a fixed time range, you can also use something like `default="2024-08-27 : 2024-09-03"`)
+```
+{% set time_filter = get_time_filter("dttm", default="Last week", remove_filter=True) %}
+SELECT
+  *,
+  '{{ time_filter.time_range }}' as time_range
+FROM logs
+WHERE
+  dttm >= {{ time_filter.from_expr }}
+  AND dttm < {{ time_filter.to_expr }}
+{% endif %}
+```
+
 **Datasets**
 
 It's possible to query physical and virtual datasets using the `dataset` macro. This is useful if you've defined computed columns and metrics on your datasets, and want to reuse the definition in adhoc SQL Lab queries.

diff --git a/superset/jinja_context.py b/superset/jinja_context.py
@@ -378,6 +378,7 @@ def get_filters(self, column: str, remove_filter: bool = False) -> list[Filter]:
     def get_time_filter(
         self,
         column: str | None = None,
+        default: str | None = None,
         target_type: str | None = None,
         remove_filter: bool = False,
     ) -> TimeFilter:
@@ -387,6 +388,8 @@ def get_time_filter(
 
         :param column: Name of the temporal column. Leave undefined to reference the
             time range from a Dashboard Native Time Range filter (when present).
+        :param default: The default value to fall back to if the time filter is
+            not present, or has the value `No 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
@@ -427,6 +430,8 @@ def get_time_filter(
                     target_type = self.table.columns_types.get(column)
 
         time_range = time_range or NO_TIME_RANGE
+        if time_range == NO_TIME_RANGE and default:
+            time_range = default
         from_expr, to_expr = get_since_until_from_time_range(time_range)
 
         def _format_dttm(dttm: datetime | None) -> str | None:

diff --git a/tests/unit_tests/jinja_context_test.py b/tests/unit_tests/jinja_context_test.py
@@ -842,9 +842,10 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
 
 
 @pytest.mark.parametrize(
-    "args,kwargs,sqlalchemy_uri,queries,time_filter,removed_filters,applied_filters",
+    "description,args,kwargs,sqlalchemy_uri,queries,time_filter,removed_filters,applied_filters",
     [
         (
+            "Missing time_range and filter will return a No filter result",
             [],
             {"target_type": "TIMESTAMP"},
             "postgresql://mydb",
@@ -858,8 +859,23 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
             [],
         ),
         (
+            "Missing time range and filter with default value will return a result with the defaults",
             [],
-            {"target_type": "TIMESTAMP"},
+            {"default": "Last week", "target_type": "TIMESTAMP"},
+            "postgresql://mydb",
+            [{}],
+            TimeFilter(
+                from_expr="TO_TIMESTAMP('2024-08-27 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
+                to_expr="TO_TIMESTAMP('2024-09-03 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US')",
+                time_range="Last week",
+            ),
+            [],
+            [],
+        ),
+        (
+            "Time range is extracted with the expected format, and default is ignored",
+            [],
+            {"default": "Last month", "target_type": "TIMESTAMP"},
             "postgresql://mydb",
             [{"time_range": "Last week"}],
             TimeFilter(
@@ -871,6 +887,7 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
             [],
         ),
         (
+            "Filter is extracted with the native format of the column (TIMESTAMP)",
             ["dttm"],
             {},
             "postgresql://mydb",
@@ -894,6 +911,7 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
             ["dttm"],
         ),
         (
+            "Filter is extracted with the native format of the column (DATE)",
             ["dt"],
             {"remove_filter": True},
             "postgresql://mydb",
@@ -917,6 +935,7 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
             ["dt"],
         ),
         (
+            "Filter is extracted with the overridden format (TIMESTAMP to DATE)",
             ["dttm"],
             {"target_type": "DATE", "remove_filter": True},
             "trino://mydb",
@@ -942,6 +961,7 @@ def test_metric_macro_no_dataset_id_with_context_chart_no_datasource_id(
     ],
 )
 def test_get_time_filter(
+    description: str,
     args: list[Any],
     kwargs: dict[str, Any],
     sqlalchemy_uri: str,
@@ -977,6 +997,6 @@ def test_get_time_filter(
             table=table,
         )
 
-        assert cache.get_time_filter(*args, **kwargs) == time_filter
+        assert cache.get_time_filter(*args, **kwargs) == time_filter, description
         assert cache.removed_filters == removed_filters
         assert cache.applied_filters == applied_filters