This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 4ee500e47bf fix(explore): render Jinja before validating legacy chart
filters (#41996)
4ee500e47bf is described below
commit 4ee500e47bf064f098f359201994279b4e3ffaeb
Author: Jean Massucatto <[email protected]>
AuthorDate: Fri Jul 24 02:05:14 2026 -0300
fix(explore): render Jinja before validating legacy chart filters (#41996)
---
superset/utils/core.py | 10 ++++--
superset/viz.py | 10 ------
tests/unit_tests/core_tests.py | 63 ++++++++++++++++++++++++++++++++++
tests/unit_tests/test_viz_query_obj.py | 57 ++++++++++++++++++++++++++++++
4 files changed, 127 insertions(+), 13 deletions(-)
diff --git a/superset/utils/core.py b/superset/utils/core.py
index cdeba702802..5810beba34a 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -96,7 +96,6 @@ from superset.exceptions import (
SupersetException,
SupersetTimeoutException,
)
-from superset.sql.parse import sanitize_clause
from superset.superset_typing import (
AdhocColumn,
AdhocMetric,
@@ -1434,12 +1433,15 @@ def convert_legacy_filters_into_adhoc( # pylint:
disable=invalid-name
def split_adhoc_filters_into_base_filters( # pylint: disable=invalid-name
form_data: FormData,
- engine: str,
+ engine: str | None = None, # pylint: disable=unused-argument
) -> None:
"""
Mutates form data to restructure the adhoc filters in the form of the
three base
filters, `where`, `having`, and `filters` which represent free form where
sql,
free form having sql, and structured where clauses.
+
+ ``engine`` is retained for backwards compatibility and is unused: clauses
are
+ validated downstream, after Jinja templates are rendered.
"""
adhoc_filters = form_data.get("adhoc_filters")
if isinstance(adhoc_filters, list):
@@ -1460,7 +1462,9 @@ def split_adhoc_filters_into_base_filters( # pylint:
disable=invalid-name
)
elif expression_type == "SQL":
sql_expression = adhoc_filter.get("sqlExpression")
- sql_expression = sanitize_clause(sql_expression, engine)
+ # keep a trailing line comment from swallowing the " AND " join
+ if sql_expression and "--" in sql_expression:
+ sql_expression = f"{sql_expression}\n"
if clause == "WHERE":
sql_where_filters.append(sql_expression)
elif clause == "HAVING":
diff --git a/superset/viz.py b/superset/viz.py
index 412436854f7..8ee02d84240 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -56,7 +56,6 @@ from superset.exceptions import (
)
from superset.extensions import cache_manager, security_manager
from superset.models.helpers import QueryResult
-from superset.sql.parse import sanitize_clause
from superset.superset_typing import (
Column,
Metric,
@@ -414,15 +413,6 @@ class BaseViz: # pylint: disable=too-many-public-methods
self.from_dttm = from_dttm
self.to_dttm = to_dttm
- # validate sql filters
- for param in ("where", "having"):
- clause = self.form_data.get(param)
- if clause:
- engine = self.datasource.database.db_engine_spec.engine
- sanitized_clause = sanitize_clause(clause, engine)
- if sanitized_clause != clause:
- self.form_data[param] = sanitized_clause
-
# extras are used to query elements specific to a datasource type
# for instance the extra where clause that applies only to Tables
extras = {
diff --git a/tests/unit_tests/core_tests.py b/tests/unit_tests/core_tests.py
index acd0501d82f..3acad5fb744 100644
--- a/tests/unit_tests/core_tests.py
+++ b/tests/unit_tests/core_tests.py
@@ -30,6 +30,7 @@ from superset.utils.core import (
get_metric_names,
get_time_filter_status,
is_adhoc_metric,
+ split_adhoc_filters_into_base_filters,
)
from tests.unit_tests.fixtures.datasets import get_dataset_mock
@@ -55,6 +56,10 @@ SQL_ADHOC_COLUMN: AdhocColumn = {
"label": "My Adhoc Column",
"sqlExpression": "case when foo = 1 then 'foo' else 'bar' end",
}
+JINJA_HAVING = (
+ "sum(price_each) > {% if filter_values('threshold')|length %} "
+ "{{ filter_values('threshold')[0] }} {% else %} 0 {% endif %}"
+)
def test_get_metric_name_saved_metric():
@@ -233,3 +238,61 @@ def test_get_time_filter_status_no_temporal_col():
}
],
)
+
+
+def test_split_adhoc_filters_joins_sql_expressions():
+ form_data = {
+ "adhoc_filters": [
+ {
+ "expressionType": "SQL",
+ "clause": "WHERE",
+ "sqlExpression": "a = 1",
+ },
+ {
+ "expressionType": "SQL",
+ "clause": "WHERE",
+ "sqlExpression": "b = 2",
+ },
+ ]
+ }
+
+ split_adhoc_filters_into_base_filters(form_data)
+
+ assert form_data["where"] == "(a = 1) AND (b = 2)"
+
+
+def test_split_adhoc_filters_preserves_jinja_templates():
+ form_data = {
+ "adhoc_filters": [
+ {
+ "expressionType": "SQL",
+ "clause": "HAVING",
+ "sqlExpression": JINJA_HAVING,
+ }
+ ]
+ }
+
+ split_adhoc_filters_into_base_filters(form_data)
+
+ assert form_data["having"] == f"({JINJA_HAVING})"
+
+
+def test_split_adhoc_filters_terminates_inline_comments():
+ form_data = {
+ "adhoc_filters": [
+ {
+ "expressionType": "SQL",
+ "clause": "WHERE",
+ "sqlExpression": "a = 1 -- comment",
+ },
+ {
+ "expressionType": "SQL",
+ "clause": "WHERE",
+ "sqlExpression": "b = 2",
+ },
+ ]
+ }
+
+ split_adhoc_filters_into_base_filters(form_data)
+
+ assert form_data["where"] == "(a = 1 -- comment\n) AND (b = 2)"
diff --git a/tests/unit_tests/test_viz_query_obj.py
b/tests/unit_tests/test_viz_query_obj.py
new file mode 100644
index 00000000000..b5c6c04f627
--- /dev/null
+++ b/tests/unit_tests/test_viz_query_obj.py
@@ -0,0 +1,57 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Behavioral tests for ``viz.BaseViz.query_obj`` covering free-form filter
+clause handling.
+"""
+
+from typing import Any
+
+from superset import viz
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+
+JINJA_HAVING = (
+ "sum(price_each) > {% if filter_values('threshold')|length %} "
+ "{{ filter_values('threshold')[0] }} {% else %} 0 {% endif %}"
+)
+
+
+def _viz(form_data: dict[str, Any]) -> viz.BaseViz:
+ database = Database(database_name="d", sqlalchemy_uri="sqlite://")
+ datasource = SqlaTable(
+ table_name="t",
+ columns=[],
+ metrics=[],
+ main_dttm_col=None,
+ database=database,
+ )
+ return viz.BaseViz(datasource=datasource, form_data=form_data)
+
+
+def test_query_obj_preserves_jinja_in_freeform_having():
+ """
+ A free-form HAVING clause containing Jinja must reach the query object
+ extras untouched: templates are only rendered (and the resulting SQL
+ validated) downstream, so validating the raw clause here would reject
+ valid templates (regression guard for premature clause validation).
+ """
+ obj = _viz({"viz_type": "table", "having": JINJA_HAVING})
+
+ query_obj = obj.query_obj()
+
+ assert query_obj["extras"]["having"] == f"({JINJA_HAVING})"
