This is an automated email from the ASF dual-hosted git repository.
msyavuz 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 cedc35e39f fix(SQLLab): remove error icon displayed when writing Jinja
SQL even when the script is correct (#36422)
cedc35e39f is described below
commit cedc35e39f5b758e35ccb661de425c775bd8792a
Author: Felipe López <[email protected]>
AuthorDate: Tue Jan 6 10:58:40 2026 -0300
fix(SQLLab): remove error icon displayed when writing Jinja SQL even when
the script is correct (#36422)
---
superset/commands/database/validate_sql.py | 44 ++++
tests/integration_tests/databases/api_tests.py | 109 ++++++++
.../commands/databases/validate_sql_test.py | 279 +++++++++++++++++++++
3 files changed, 432 insertions(+)
diff --git a/superset/commands/database/validate_sql.py
b/superset/commands/database/validate_sql.py
index c252cf6fdb..d7ab40f882 100644
--- a/superset/commands/database/validate_sql.py
+++ b/superset/commands/database/validate_sql.py
@@ -32,6 +32,11 @@ from superset.commands.database.exceptions import (
)
from superset.daos.database import DatabaseDAO
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import (
+ SupersetSyntaxErrorException,
+ SupersetTemplateException,
+)
+from superset.jinja_context import get_template_processor
from superset.models.core import Database
from superset.sql_validators import get_validator_by_name
from superset.sql_validators.base import BaseSQLValidator
@@ -62,12 +67,51 @@ class ValidateSQLCommand(BaseCommand):
sql = self._properties["sql"]
catalog = self._properties.get("catalog")
schema = self._properties.get("schema")
+ template_params = self._properties.get("template_params") or {}
+
try:
+ # Render Jinja templates to handle template syntax before
+ # validation. Note: The ENABLE_TEMPLATE_PROCESSING feature flag is
+ # checked within get_template_processor(), which returns
+ # NoOpTemplateProcessor when disabled. Template processing errors
+ # (e.g., undefined filters, syntax errors) are caught by this
+ # exception handler and surfaced to the client as
+ # ValidatorSQLError or ValidatorSQL400Error with appropriate error
+ # messages.
+ template_processor = get_template_processor(self._model)
+ # process_template() renders Jinja2 templates and always returns a
+ # new string (does not mutate the input SQL). May raise
+ # SupersetSyntaxErrorException for template syntax errors or
+ # SupersetTemplateException for internal errors.
+ sql = template_processor.process_template(sql, **template_params)
+
timeout = app.config["SQLLAB_VALIDATION_TIMEOUT"]
timeout_msg = f"The query exceeded the {timeout} seconds timeout."
with utils.timeout(seconds=timeout, error_message=timeout_msg):
errors = self._validator.validate(sql, catalog, schema,
self._model)
return [err.to_dict() for err in errors]
+ except SupersetSyntaxErrorException as ex:
+ # Template syntax errors (e.g., invalid Jinja2 syntax, undefined
variables)
+ # These contain detailed error information including line numbers
+ logger.warning(
+ "Template syntax error during SQL validation",
+ extra={"errors": [err.message for err in ex.errors]},
+ )
+ raise ValidatorSQL400Error(ex.errors[0]) from ex
+ except SupersetTemplateException as ex:
+ # Internal template processing errors (e.g., recursion, unexpected
failures)
+ logger.error(
+ "Template processing error during SQL validation",
exc_info=True
+ )
+ superset_error = SupersetError(
+ message=__(
+ "Template processing failed: %(ex)s",
+ ex=str(ex),
+ ),
+ error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ raise ValidatorSQL400Error(superset_error) from ex
except Exception as ex:
logger.exception(ex)
superset_error = SupersetError(
diff --git a/tests/integration_tests/databases/api_tests.py
b/tests/integration_tests/databases/api_tests.py
index cfad961589..94b24896a5 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -4104,6 +4104,115 @@ class TestDatabaseApi(SupersetTestCase):
assert rv.status_code == 422
assert "Kaboom!" in response["errors"][0]["message"]
+ @mock.patch.dict(
+ "superset.config.SQL_VALIDATORS_BY_ENGINE",
+ SQL_VALIDATORS_BY_ENGINE,
+ clear=True,
+ )
+ def test_validate_sql_with_jinja_templates(self):
+ """
+ Database API: validate SQL with Jinja templates
+ """
+ request_payload = {
+ "sql": (
+ "SELECT *\nFROM birth_names\nWHERE 1=1\n"
+ "{% if city_filter is defined %}\n"
+ " AND city = '{{ city_filter }}'\n{% endif %}\n"
+ "LIMIT {{ limit | default(100) }}"
+ ),
+ "schema": None,
+ "template_params": {},
+ }
+
+ example_db = get_example_database()
+ if example_db.backend not in ("presto", "postgresql"):
+ pytest.skip("Only presto and PG are implemented")
+
+ self.login(ADMIN_USERNAME)
+ uri = f"api/v1/database/{example_db.id}/validate_sql/"
+ rv = self.client.post(uri, json=request_payload)
+ response = json.loads(rv.data.decode("utf-8"))
+ assert rv.status_code == 200
+ # Template was successfully rendered and validated
+ # so a valid query returns an empty result list
+ result = response["result"]
+ assert isinstance(result, list)
+ assert len(result) == 0
+
+ @mock.patch.dict(
+ "superset.config.SQL_VALIDATORS_BY_ENGINE",
+ SQL_VALIDATORS_BY_ENGINE,
+ clear=True,
+ )
+ def test_validate_sql_with_jinja_templates_and_params(self):
+ """
+ Database API: validate SQL with Jinja templates and parameters
+ """
+ request_payload = {
+ "sql": (
+ "SELECT *\nFROM birth_names\nWHERE 1=1\n"
+ "{% if city_filter is defined %}\n"
+ " AND city = '{{ city_filter }}'\n"
+ "{% endif %}\nLIMIT {{ limit }}"
+ ),
+ "schema": None,
+ "template_params": {"city_filter": "New York", "limit": 50},
+ }
+
+ example_db = get_example_database()
+ if example_db.backend not in ("presto", "postgresql"):
+ pytest.skip("Only presto and PG are implemented")
+
+ self.login(ADMIN_USERNAME)
+ uri = f"api/v1/database/{example_db.id}/validate_sql/"
+ rv = self.client.post(uri, json=request_payload)
+ response = json.loads(rv.data.decode("utf-8"))
+ assert rv.status_code == 200
+ # Template was successfully rendered with parameters and validated
+ # so a valid query returns an empty result list
+ result = response["result"]
+ assert isinstance(result, list)
+ assert len(result) == 0
+
+ @mock.patch.dict(
+ "superset.config.SQL_VALIDATORS_BY_ENGINE",
+ SQL_VALIDATORS_BY_ENGINE,
+ clear=True,
+ )
+ def test_validate_sql_with_jinja_invalid_sql_after_render(self):
+ """
+ Database API: validate SQL with Jinja templates that renders to
invalid SQL
+
+ This test ensures that SQL validation errors are not hidden by template
+ processing. The template should render successfully, but the resulting
SQL
+ should fail syntax validation.
+ """
+ request_payload = {
+ "sql": (
+ "SELECT *\nFROM birth_names\n"
+ "{% if add_invalid_clause %}\n"
+ "WHERE\n"
+ "{% endif %}"
+ ),
+ "schema": None,
+ "template_params": {"add_invalid_clause": True},
+ }
+
+ example_db = get_example_database()
+ if example_db.backend not in ("presto", "postgresql"):
+ pytest.skip("Only presto and PG are implemented")
+
+ self.login(ADMIN_USERNAME)
+ uri = f"api/v1/database/{example_db.id}/validate_sql/"
+ rv = self.client.post(uri, json=request_payload)
+ response = json.loads(rv.data.decode("utf-8"))
+ assert rv.status_code == 200
+ # The template should render successfully, but SQL validation
+ # should catch the syntax error (WHERE clause with no condition)
+ result = response["result"]
+ assert isinstance(result, list)
+ assert len(result) > 0
+
def test_get_databases_with_extra_filters(self):
"""
API: Test get database with extra query filter.
diff --git a/tests/unit_tests/commands/databases/validate_sql_test.py
b/tests/unit_tests/commands/databases/validate_sql_test.py
new file mode 100644
index 0000000000..46df10cecf
--- /dev/null
+++ b/tests/unit_tests/commands/databases/validate_sql_test.py
@@ -0,0 +1,279 @@
+# 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.
+
+from typing import Any
+from unittest.mock import MagicMock
+
+import pytest
+from pytest_mock import MockerFixture
+
+from superset.commands.database.exceptions import (
+ ValidatorSQL400Error,
+ ValidatorSQLError,
+)
+from superset.commands.database.validate_sql import ValidateSQLCommand
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import (
+ SupersetSyntaxErrorException,
+ SupersetTemplateException,
+)
+
+
[email protected]
+def mock_database(mocker: MockerFixture) -> MagicMock:
+ """Create a mock database with PostgreSQL engine."""
+ database = mocker.MagicMock()
+ database.id = 1
+ database.db_engine_spec.engine = "postgresql"
+
+ DatabaseDAO = mocker.patch( # noqa: N806
+ "superset.commands.database.validate_sql.DatabaseDAO"
+ )
+ DatabaseDAO.find_by_id.return_value = database
+ return database
+
+
[email protected]
+def mock_validator(mocker: MockerFixture) -> MagicMock:
+ """Create a mock SQL validator."""
+ validator = mocker.MagicMock()
+ validator.name = "PostgreSQLValidator"
+ validator.validate.return_value = []
+
+ get_validator_by_name = mocker.patch(
+ "superset.commands.database.validate_sql.get_validator_by_name"
+ )
+ get_validator_by_name.return_value = validator
+ return validator
+
+
[email protected]
+def mock_config(mocker: MockerFixture) -> dict[str, Any]:
+ """Mock the application config."""
+ config = {
+ "SQL_VALIDATORS_BY_ENGINE": {"postgresql": "PostgreSQLValidator"},
+ "SQLLAB_VALIDATION_TIMEOUT": 30,
+ }
+ mocker.patch("superset.commands.database.validate_sql.app.config", config)
+ return config
+
+
[email protected]
+def mock_template_processor(
+ mocker: MockerFixture, mock_database: MagicMock
+) -> MagicMock:
+ """Create a mock template processor."""
+ template_processor = mocker.MagicMock()
+ get_template_processor = mocker.patch(
+ "superset.commands.database.validate_sql.get_template_processor"
+ )
+ get_template_processor.return_value = template_processor
+ return template_processor
+
+
+def test_validate_sql_with_jinja_templates(
+ mock_database: MagicMock,
+ mock_validator: MagicMock,
+ mock_template_processor: MagicMock,
+ mock_config: dict[str, Any],
+) -> None:
+ """Test that Jinja templates are rendered before SQL validation."""
+ sql_with_jinja = """SELECT *
+ FROM birth_names
+ WHERE 1=1
+ {% if city_filter is defined %}
+ AND city = '{{ city_filter }}'
+ {% endif %}
+ LIMIT {{ limit | default(100) }}"""
+
+ mock_template_processor.process_template.return_value = (
+ "SELECT *\nFROM birth_names\nWHERE 1=1\nLIMIT 100"
+ )
+
+ data = {"sql": sql_with_jinja, "schema": "public", "template_params": {}}
+ command = ValidateSQLCommand(model_id=1, data=data)
+ result = command.run()
+
+
mock_template_processor.process_template.assert_called_once_with(sql_with_jinja)
+ mock_validator.validate.assert_called_once()
+ assert result == []
+
+
+def test_validate_sql_with_jinja_templates_and_params(
+ mock_database: MagicMock,
+ mock_validator: MagicMock,
+ mock_template_processor: MagicMock,
+ mock_config: dict[str, Any],
+) -> None:
+ """Test that Jinja templates are rendered with parameters before SQL
validation."""
+ sql_with_jinja = """SELECT *
+ FROM birth_names
+ WHERE 1=1
+ {% if city_filter is defined %}
+ AND city = '{{ city_filter }}'
+ {% endif %}
+ LIMIT {{ limit }}"""
+
+ template_params = {"city_filter": "New York", "limit": 50}
+ mock_template_processor.process_template.return_value = (
+ "SELECT *\nFROM birth_names\nWHERE 1=1\n AND city = 'New
York'\nLIMIT 50"
+ )
+
+ data = {
+ "sql": sql_with_jinja,
+ "schema": "public",
+ "template_params": template_params,
+ }
+ command = ValidateSQLCommand(model_id=1, data=data)
+ result = command.run()
+
+ mock_template_processor.process_template.assert_called_once_with(
+ sql_with_jinja, **template_params
+ )
+ mock_validator.validate.assert_called_once()
+ assert result == []
+
+
+def test_validate_sql_without_jinja_templates(
+ mock_database: MagicMock,
+ mock_validator: MagicMock,
+ mock_template_processor: MagicMock,
+ mock_config: dict[str, Any],
+) -> None:
+ """Test that regular SQL without Jinja templates still works."""
+ simple_sql = "SELECT * FROM birth_names LIMIT 100"
+ mock_template_processor.process_template.return_value = simple_sql
+
+ data = {"sql": simple_sql, "schema": "public", "template_params": {}}
+ command = ValidateSQLCommand(model_id=1, data=data)
+ result = command.run()
+
+ mock_template_processor.process_template.assert_called_once()
+ mock_validator.validate.assert_called_once()
+ assert result == []
+
+
+def test_validate_sql_template_syntax_error(
+ mock_database: MagicMock,
+ mock_validator: MagicMock,
+ mock_template_processor: MagicMock,
+ mock_config: dict[str, Any],
+) -> None:
+ """
+ Test that template syntax errors are properly surfaced to the client.
+
+ When template processing raises a SupersetSyntaxErrorException (e.g.,
+ invalid Jinja2 syntax, undefined variables), it should be caught and
+ converted to a ValidatorSQL400Error with detailed error information
+ including line numbers.
+ """
+ syntax_error = SupersetError(
+ message="Jinja2 template error (UndefinedError): 'city_filter' is
undefined",
+ error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
+ level=ErrorLevel.ERROR,
+ extra={"template": "SELECT * FROM...", "line": 3},
+ )
+ mock_template_processor.process_template.side_effect =
SupersetSyntaxErrorException(
+ [syntax_error]
+ )
+
+ sql_with_undefined_var = """SELECT *
+ FROM birth_names
+ WHERE city = '{{ city_filter }}'
+ LIMIT 100"""
+
+ data = {
+ "sql": sql_with_undefined_var,
+ "schema": "public",
+ "template_params": {},
+ }
+ command = ValidateSQLCommand(model_id=1, data=data)
+
+ with pytest.raises(ValidatorSQL400Error) as exc_info:
+ command.run()
+
+ error = exc_info.value
+ assert error.error.message is not None
+ assert "'city_filter' is undefined" in error.error.message
+ mock_validator.validate.assert_not_called()
+
+
+def test_validate_sql_template_processing_error(
+ mock_database: MagicMock,
+ mock_validator: MagicMock,
+ mock_template_processor: MagicMock,
+ mock_config: dict[str, Any],
+) -> None:
+ """
+ Test that internal template processing errors are properly surfaced to the
client.
+
+ When template processing raises a SupersetTemplateException (e.g.,
recursion,
+ unexpected failures), it should be caught and converted to a
ValidatorSQL400Error
+ with an appropriate error message.
+ """
+ mock_template_processor.process_template.side_effect =
SupersetTemplateException(
+ "Infinite recursion detected in template"
+ )
+
+ data = {
+ "sql": "SELECT * FROM birth_names LIMIT 100",
+ "schema": "public",
+ "template_params": {},
+ }
+ command = ValidateSQLCommand(model_id=1, data=data)
+
+ with pytest.raises(ValidatorSQL400Error) as exc_info:
+ command.run()
+
+ error = exc_info.value
+ assert error.error.message is not None
+ assert "Template processing failed" in error.error.message
+ assert "Infinite recursion" in error.error.message
+ mock_validator.validate.assert_not_called()
+
+
+def test_validate_sql_generic_exception(
+ mock_database: MagicMock,
+ mock_validator: MagicMock,
+ mock_template_processor: MagicMock,
+ mock_config: dict[str, Any],
+) -> None:
+ """
+ Test that unexpected exceptions are still caught and handled gracefully.
+
+ When an unexpected exception occurs (not template-related), it should be
caught
+ and converted to a ValidatorSQLError with the validator name in the
message.
+ """
+ mock_template_processor.process_template.side_effect = RuntimeError(
+ "Unexpected error occurred"
+ )
+
+ data = {
+ "sql": "SELECT * FROM birth_names",
+ "schema": "public",
+ "template_params": {},
+ }
+ command = ValidateSQLCommand(model_id=1, data=data)
+
+ with pytest.raises(ValidatorSQLError) as exc_info:
+ command.run()
+
+ error = exc_info.value
+ assert error.error.message is not None
+ assert "PostgreSQLValidator" in error.error.message
+ assert "Unexpected error occurred" in error.error.message
+ mock_validator.validate.assert_not_called()
