fix(SQLLab): remove error icon displayed when writing Jinja SQL even when the script is correct (#36422)

Author: FelipeGLopez

superset/commands/database/validate_sql.py

@@ -32,6 +32,11 @@
 )
 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 @@ def run(self) -> list[dict[str, Any]]:
         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(

tests/integration_tests/databases/api_tests.py

@@ -4104,6 +4104,115 @@ def test_validate_sql_endpoint_failure(self, get_validator_by_name):
         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.