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:39:37 UTC
(superset) branch 3.1 updated (e48dae781f -> 19b05d3422)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
from e48dae781f fix(sqllab): unable to remove table (#27636)
new 119b2b0c93 fix: Leverage actual database for rendering Jinjarized SQL (#27646)
new 19b05d3422 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) 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 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 19b05d3422874f2ca11d00ec6fdd5bb914371e52
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 3841477cb7..63684f9f85 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 == []
(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 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 119b2b0c9389ef394b1e2dbbc202f5a77fd19dc6
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 2fd0d2dc76..3841477cb7 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 3c4b4106d3..19e1ab8d2e 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 42b796bc58..1d4f4088cd 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
@@ -1913,6 +1914,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
)