Skip to content

fix(sql-lab): raise SupersetTemplateException instead of raw UndefinedError in SQLExecutor.execute_async #42851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eschutho wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −1
Open

fix(sql-lab): raise SupersetTemplateException instead of raw UndefinedError in SQLExecutor.execute_async #42851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion superset/sql/execution/executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@

from flask import current_app as app, g, has_app_context
from flask_babel import gettext as __
from jinja2.exceptions import TemplateError

from superset import db
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
Expand All @@ -75,6 +76,7 @@
SupersetErrorException,
SupersetParseError,
SupersetSecurityException,
SupersetTemplateException,
SupersetTimeoutException,
)
from superset.extensions import cache_manager
Expand Down Expand Up @@ -751,14 +753,18 @@ def _render_sql_template(
:param sql: SQL string potentially containing Jinja2 templates
:param template_params: Parameters to pass to the template
:returns: Rendered SQL string
:raises SupersetTemplateException: if the template fails to render
"""
if template_params is None:
return sql

from superset.jinja_context import get_template_processor

tp = get_template_processor(database=self.database)
return tp.process_template(sql, **template_params)
try:
return tp.process_template(sql, **template_params)
except TemplateError as ex:
raise SupersetTemplateException(str(ex)) from ex
Comment on lines +764 to +767

@codeant-ai-for-open-source codeant-ai-for-open-source Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The broad TemplateError catch flattens compilation-time TemplateSyntaxError exceptions raised by direct-rendering processors such as Spark and Trino into a generic SupersetTemplateException, discarding the structured SupersetSyntaxErrorException/SupersetError details that the standard processor provides. Preserve the existing typed syntax-error mapping before applying the fallback wrapper for raw render-time errors. [api mismatch]

Severity Level: Major ⚠️ 
- ⚠️ Spark and Trino template syntax errors lose line metadata.
- ⚠️ API responses lose structured syntax-error classification.
- ⚠️ SQL Lab users receive less actionable template diagnostics.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** superset/sql/execution/executor.py
**Line:** 764:767
**Comment:**
	*Api Mismatch: The broad `TemplateError` catch flattens compilation-time `TemplateSyntaxError` exceptions raised by direct-rendering processors such as Spark and Trino into a generic `SupersetTemplateException`, discarding the structured `SupersetSyntaxErrorException`/`SupersetError` details that the standard processor provides. Preserve the existing typed syntax-error mapping before applying the fallback wrapper for raw render-time errors.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


def _apply_limit_to_script(self, script: SQLScript, opts: QueryOptions) -> None:
"""
Expand Down
37 changes: 37 additions & 0 deletions tests/unit_tests/sql/execution/test_executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
)

from superset.models.core import Database
from tests.unit_tests.conftest import with_feature_flags

# Note: database, database_with_dml, mock_db_session fixtures and
# mock_query_execution helper are imported from conftest.py
Expand Down Expand Up @@ -789,6 +790,42 @@ def test_execute_async_dml_without_permission_raises(
database.execute_async("INSERT INTO users (name) VALUES ('test')")


@with_feature_flags(ENABLE_TEMPLATE_PROCESSING=True)
def test_execute_async_undefined_template_var_raises_superset_template_exception(
mocker: MockerFixture, database: Database, app_context: None
) -> None:
"""A Jinja template referencing an undefined variable (not called as a
function) must not leak a raw ``jinja2.exceptions.UndefinedError`` out of
``execute_async`` - it should surface as ``SupersetTemplateException``."""
from superset.exceptions import SupersetTemplateException

mocker.patch.dict(
current_app.config, {"SQL_QUERY_MUTATOR": None, "SQLLAB_TIMEOUT": 30}
)

options = QueryOptions(template_params={"foo": "bar"})

with pytest.raises(SupersetTemplateException):
database.execute_async("SELECT {{ missing_var[0] }}", options=options)


@with_feature_flags(ENABLE_TEMPLATE_PROCESSING=True)
def test_execute_sync_undefined_template_var_returns_failed_result(
mocker: MockerFixture, database: Database, app_context: None
) -> None:
"""The sync ``execute`` path's broad ``except Exception`` still catches the
template rendering failure and returns a FAILED ``QueryResult``, unchanged
by the ``_render_sql_template`` fix."""
mocker.patch.dict(
current_app.config, {"SQL_QUERY_MUTATOR": None, "SQLLAB_TIMEOUT": 30}
)

options = QueryOptions(template_params={"foo": "bar"})
result = database.execute("SELECT {{ missing_var[0] }}", options=options)

assert result.status == QueryStatus.FAILED


def test_async_handle_get_status(
mocker: MockerFixture,
database: Database,
Expand Down
Loading