You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/26 19:37:53 UTC
(superset) branch 4.0 updated (34b06f94ab -> 5c567895cc)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
from 34b06f94ab fix(AlertReports): disabling value when not null option is active (#27550)
new 51ad63426c fix: Leverage actual database for rendering Jinjarized SQL (#27646)
new 5c567895cc fix: Provide more inclusive error handling for saved queries (#27644)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
superset/models/sql_lab.py | 5 +--
superset/security/manager.py | 4 +--
superset/sql_parse.py | 16 +++++----
tests/unit_tests/models/sql_lab_test.py | 59 +++++++++++++++++++++++++++++++++
tests/unit_tests/sql_parse_tests.py | 6 +++-
5 files changed, 77 insertions(+), 13 deletions(-)
create mode 100644 tests/unit_tests/models/sql_lab_test.py
(superset) 01/02: fix: Leverage actual database for rendering Jinjarized SQL (#27646)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 51ad63426c3ad3e227b3393cbc377621f98f8f89
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Wed Mar 27 08:12:25 2024 +1300
fix: Leverage actual database for rendering Jinjarized SQL (#27646)
---
superset/models/sql_lab.py | 2 +-
superset/security/manager.py | 4 +---
superset/sql_parse.py | 15 ++++++++-------
tests/unit_tests/sql_parse_tests.py | 6 +++++-
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py
index 2d7384a74e..f22d774e88 100644
--- a/superset/models/sql_lab.py
+++ b/superset/models/sql_lab.py
@@ -73,7 +73,7 @@ class SqlTablesMixin: # pylint: disable=too-few-public-methods
return list(
extract_tables_from_jinja_sql(
self.sql, # type: ignore
- self.database.db_engine_spec.engine, # type: ignore
+ self.database, # type: ignore
)
)
except SupersetSecurityException:
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 2833e88645..e5a32e97a7 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -1963,9 +1963,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
default_schema = database.get_default_schema_for_query(query)
tables = {
Table(table_.table, table_.schema or default_schema)
- for table_ in extract_tables_from_jinja_sql(
- query.sql, database.db_engine_spec.engine
- )
+ for table_ in extract_tables_from_jinja_sql(query.sql, database)
}
elif table:
tables = {table}
diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index 58bca48a6e..6df5dbc089 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -23,8 +23,7 @@ import re
import urllib.parse
from collections.abc import Iterable, Iterator
from dataclasses import dataclass
-from typing import Any, cast
-from unittest.mock import Mock
+from typing import Any, cast, TYPE_CHECKING
import sqlparse
from flask_babel import gettext as __
@@ -71,6 +70,9 @@ try:
except (ImportError, ModuleNotFoundError):
sqloxide_parse = None
+if TYPE_CHECKING:
+ from superset.models.core import Database
+
RESULT_OPERATIONS = {"UNION", "INTERSECT", "EXCEPT", "SELECT"}
ON_KEYWORD = "ON"
PRECEDES_TABLE_NAME = {"FROM", "JOIN", "DESCRIBE", "WITH", "LEFT JOIN", "RIGHT JOIN"}
@@ -1054,7 +1056,7 @@ def extract_table_references(
}
-def extract_tables_from_jinja_sql(sql: str, engine: str | None = None) -> set[Table]:
+def extract_tables_from_jinja_sql(sql: str, database: Database) -> set[Table]:
"""
Extract all table references in the Jinjafied SQL statement.
@@ -1067,7 +1069,7 @@ def extract_tables_from_jinja_sql(sql: str, engine: str | None = None) -> set[Ta
SQLGlot.
:param sql: The Jinjafied SQL statement
- :param engine: The associated database engine
+ :param database: The database associated with the SQL statement
:returns: The set of tables referenced in the SQL statement
:raises SupersetSecurityException: If SQLGlot is unable to parse the SQL statement
"""
@@ -1076,8 +1078,7 @@ def extract_tables_from_jinja_sql(sql: str, engine: str | None = None) -> set[Ta
get_template_processor,
)
- # Mock the required database as the processor signature is exposed publically.
- processor = get_template_processor(database=Mock(backend=engine))
+ processor = get_template_processor(database)
template = processor.env.parse(sql)
tables = set()
@@ -1107,6 +1108,6 @@ def extract_tables_from_jinja_sql(sql: str, engine: str | None = None) -> set[Ta
tables
| ParsedQuery(
sql_statement=processor.process_template(template),
- engine=engine,
+ engine=database.db_engine_spec.engine,
).tables
)
diff --git a/tests/unit_tests/sql_parse_tests.py b/tests/unit_tests/sql_parse_tests.py
index 81ea0e5a7a..dab5dbf9c7 100644
--- a/tests/unit_tests/sql_parse_tests.py
+++ b/tests/unit_tests/sql_parse_tests.py
@@ -17,6 +17,7 @@
# pylint: disable=invalid-name, redefined-outer-name, too-many-lines
from typing import Optional
+from unittest.mock import Mock
import pytest
import sqlparse
@@ -1912,6 +1913,9 @@ def test_extract_tables_from_jinja_sql(
expected: set[Table],
) -> None:
assert (
- extract_tables_from_jinja_sql(sql.format(engine=engine, macro=macro), engine)
+ extract_tables_from_jinja_sql(
+ sql=sql.format(engine=engine, macro=macro),
+ database=Mock(),
+ )
== expected
)
(superset) 02/02: fix: Provide more inclusive error handling for saved queries (#27644)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5c567895cce0a0e4e9640bd5ffa198040435d045
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Wed Mar 27 08:22:14 2024 +1300
fix: Provide more inclusive error handling for saved queries (#27644)
(cherry picked from commit 3ae74d1f2daf0399434e16145ba585045bff779f)
---
superset/models/sql_lab.py | 3 +-
superset/sql_parse.py | 1 +
tests/unit_tests/models/sql_lab_test.py | 59 +++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py
index f22d774e88..25c21cdfc8 100644
--- a/superset/models/sql_lab.py
+++ b/superset/models/sql_lab.py
@@ -30,6 +30,7 @@ from flask_appbuilder import Model
from flask_appbuilder.models.decorators import renders
from flask_babel import gettext as __
from humanize import naturaltime
+from jinja2.exceptions import TemplateError
from sqlalchemy import (
Boolean,
Column,
@@ -76,7 +77,7 @@ class SqlTablesMixin: # pylint: disable=too-few-public-methods
self.database, # type: ignore
)
)
- except SupersetSecurityException:
+ except (SupersetSecurityException, TemplateError):
return []
diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index 6df5dbc089..62a2457171 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -1072,6 +1072,7 @@ def extract_tables_from_jinja_sql(sql: str, database: Database) -> set[Table]:
:param database: The database associated with the SQL statement
:returns: The set of tables referenced in the SQL statement
:raises SupersetSecurityException: If SQLGlot is unable to parse the SQL statement
+ :raises jinja2.exceptions.TemplateError: If the Jinjafied SQL could not be rendered
"""
from superset.jinja_context import ( # pylint: disable=import-outside-toplevel
diff --git a/tests/unit_tests/models/sql_lab_test.py b/tests/unit_tests/models/sql_lab_test.py
new file mode 100644
index 0000000000..fb5db97474
--- /dev/null
+++ b/tests/unit_tests/models/sql_lab_test.py
@@ -0,0 +1,59 @@
+# 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 unittest.mock import MagicMock
+
+import pytest
+from flask_appbuilder import Model
+from jinja2.exceptions import TemplateError
+from pytest_mock import MockFixture
+
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import SupersetSecurityException
+from superset.models.sql_lab import Query, SavedQuery, SqlTablesMixin
+
+
+@pytest.mark.parametrize(
+ "klass",
+ [
+ Query,
+ SavedQuery,
+ ],
+)
+@pytest.mark.parametrize(
+ "exception",
+ [
+ SupersetSecurityException(
+ SupersetError(
+ error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
+ message="",
+ level=ErrorLevel.ERROR,
+ )
+ ),
+ TemplateError,
+ ],
+)
+def test_sql_tables_mixin_sql_tables_exception(
+ klass: type[Model],
+ exception: Exception,
+ mocker: MockFixture,
+) -> None:
+ mocker.patch(
+ "superset.models.sql_lab.extract_tables_from_jinja_sql",
+ side_effect=exception,
+ )
+
+ assert klass(sql="SELECT 1", database=MagicMock()).sql_tables == []