iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat: implement specific errors for SQL Lab (#15206) 

* RESULTS_BACKEND_NOT_CONFIGURED_ERROR

* DML_NOT_ALLOWED_ERROR

* INVALID_CxAS_QUERY_ERROR

* Fix lint

* Add more tests
This commit is contained in:
Beto Dealmeida 2021-06-23 07:58:20 -07:00 committed by GitHub
parent 216e2b8e8e
commit 4b00c152cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 281 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
docs/src/pages/docs/Miscellaneous/issue_codes.mdx
View File

@ -207,3 +207,43 @@ The submitted payload has the incorrect schema.
```
Please check that the request payload has the expected schema.
## Issue 1021
```
Results backend needed for asynchronous queries is not configured.
```
Your instance of Superset doesn't have a results backend configured, which is needed for asynchronous queries. Please contact an administrator for further assistance.
## Issue 1022
```
Database does not allow data manipulation.
```
Only `SELECT` statements are allowed against this database. Please contact an administrator if you need to run DML (data manipulation language) on this database.
## Issue 1023
```
CTAS (create table as select) doesn't have a SELECT statement at the end.
```
The last statement in a query run as CTAS (create table as select) MUST be a SELECT statement. Please make sure the last statement in the query is a SELECT.
## Issue 1024
```
CVAS (create view as select) query has more than one statement.
```
When running a CVAS (create view as select) the query should have a single statement. Please make sure the query has a single statement, and no extra semi-colons other than the last one.
## Issue 1025
```
CVAS (create view as select) query is not a SELECT statement.
```
When running a CVAS (create view as select) the query should be a SELECT statement. Please make sure the query has a single statement and it's a SELECT statement.

4
superset-frontend/src/components/ErrorMessage/types.ts
View File

@ -57,6 +57,10 @@ export const ErrorTypeEnum = {
// Sqllab error
MISSING_TEMPLATE_PARAMS_ERROR: 'MISSING_TEMPLATE_PARAMS_ERROR',
RESULTS_BACKEND_NOT_CONFIGURED_ERROR: 'RESULTS_BACKEND_NOT_CONFIGURED_ERROR',
DML_NOT_ALLOWED_ERROR: 'DML_NOT_ALLOWED_ERROR',
INVALID_CTAS_QUERY_ERROR: 'INVALID_CTAS_QUERY_ERROR',
INVALID_CVAS_QUERY_ERROR: 'INVALID_CVAS_QUERY_ERROR',
// Generic errors
GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR',

16
superset-frontend/src/setup/setupErrorMessages.ts
View File

@ -51,6 +51,22 @@ export default function setupErrorMessages() {
ErrorTypeEnum.MISSING_TEMPLATE_PARAMS_ERROR,
ParameterErrorMessage,
);
errorMessageComponentRegistry.registerValue(
ErrorTypeEnum.RESULTS_BACKEND_NOT_CONFIGURED_ERROR,
DatabaseErrorMessage,
);
errorMessageComponentRegistry.registerValue(
ErrorTypeEnum.DML_NOT_ALLOWED_ERROR,
DatabaseErrorMessage,
);
errorMessageComponentRegistry.registerValue(
ErrorTypeEnum.INVALID_CTAS_QUERY_ERROR,
DatabaseErrorMessage,
);
errorMessageComponentRegistry.registerValue(
ErrorTypeEnum.INVALID_CVAS_QUERY_ERROR,
DatabaseErrorMessage,
);
errorMessageComponentRegistry.registerValue(
ErrorTypeEnum.CONNECTION_INVALID_HOSTNAME_ERROR,
DatabaseErrorMessage,

45
superset/errors.py
View File

@ -66,6 +66,10 @@ class SupersetErrorType(str, Enum):
# Sql Lab errors
MISSING_TEMPLATE_PARAMS_ERROR = "MISSING_TEMPLATE_PARAMS_ERROR"
RESULTS_BACKEND_NOT_CONFIGURED_ERROR = "RESULTS_BACKEND_NOT_CONFIGURED_ERROR"
DML_NOT_ALLOWED_ERROR = "DML_NOT_ALLOWED_ERROR"
INVALID_CTAS_QUERY_ERROR = "INVALID_CTAS_QUERY_ERROR"
INVALID_CVAS_QUERY_ERROR = "INVALID_CVAS_QUERY_ERROR"
# Generic errors
GENERIC_COMMAND_ERROR = "GENERIC_COMMAND_ERROR"
@ -147,6 +151,21 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
),
},
],
SupersetErrorType.RESULTS_BACKEND_NOT_CONFIGURED_ERROR: [
{
"code": 1021,
"message": _(
"Issue 1021 - Results backend needed for asynchronous queries "
"is not configured."
),
},
],
SupersetErrorType.DML_NOT_ALLOWED_ERROR: [
{
"code": 1022,
"message": _("Issue 1022 - Database does not allow data manipulation."),
},
],
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR: [
{
"code": 1007,
@ -250,6 +269,32 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
),
}
],
SupersetErrorType.INVALID_CTAS_QUERY_ERROR: [
{
"code": 1023,
"message": _(
"Issue 1023 - The CTAS (create table as select) doesn't have a "
"SELECT statement at the end. Please make sure your query has a "
"SELECT as its last statement. Then, try running your query again."
),
},
],
SupersetErrorType.INVALID_CVAS_QUERY_ERROR: [
{
"code": 1024,
"message": _(
"Issue 1024 - CVAS (create view as select) query has more than "
"one statement."
),
},
{
"code": 1025,
"message": _(
"Issue 1025 - CVAS (create view as select) query is not a "
"SELECT statement."
),
},
],
}

85
superset/sql_lab.py
View File

@ -28,13 +28,15 @@ import pyarrow as pa
import simplejson as json
from celery import Task
from celery.exceptions import SoftTimeLimitExceeded
from flask_babel import lazy_gettext as _
from flask_babel import gettext as __, lazy_gettext as _
from sqlalchemy.orm import Session
from werkzeug.local import LocalProxy
from superset import app, results_backend, results_backend_use_msgpack, security_manager
from superset.dataframe import df_to_records
from superset.db_engine_specs import BaseEngineSpec
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetErrorException, SupersetErrorsException
from superset.extensions import celery_app
from superset.models.core import Database
from superset.models.sql_lab import LimitingFactor, Query
@ -86,25 +88,34 @@ class SqlLabTimeoutException(SqlLabException):
def handle_query_error(
msg: str, query: Query, session: Session, payload: Optional[Dict[str, Any]] = None
ex: Exception,
query: Query,
session: Session,
payload: Optional[Dict[str, Any]] = None,
prefix_message: str = "",
) -> Dict[str, Any]:
"""Local method handling error while processing the SQL"""
payload = payload or {}
msg = f"{prefix_message} {str(ex)}".strip()
troubleshooting_link = config["TROUBLESHOOTING_LINK"]
query.error_message = msg
query.status = QueryStatus.FAILED
query.tmp_table_name = None
# extract DB-specific errors (invalid column, eg)
errors = [
dataclasses.asdict(error)
for error in query.database.db_engine_spec.extract_errors(msg)
]
if isinstance(ex, SupersetErrorException):
errors = [ex.error]
elif isinstance(ex, SupersetErrorsException):
errors = ex.errors
else:
errors = query.database.db_engine_spec.extract_errors(str(ex))
errors_payload = [dataclasses.asdict(error) for error in errors]
if errors:
query.set_extra_json_key("errors", errors)
query.set_extra_json_key("errors", errors_payload)
session.commit()
payload.update({"status": query.status, "error": msg, "errors": errors})
payload.update({"status": query.status, "error": msg, "errors": errors_payload})
if troubleshooting_link:
payload["link"] = troubleshooting_link
return payload
@ -186,7 +197,7 @@ def get_sql_results( # pylint: disable=too-many-arguments
logger.debug("Query %d: %s", query_id, ex)
stats_logger.incr("error_sqllab_unhandled")
query = get_query(query_id, session)
return handle_query_error(str(ex), query, session)
return handle_query_error(ex, query, session)
# pylint: disable=too-many-arguments, too-many-locals
@ -210,8 +221,12 @@ def execute_sql_statement(
increased_limit = None if query.limit is None else query.limit + 1
if not db_engine_spec.is_readonly_query(parsed_query) and not database.allow_dml:
raise SqlLabSecurityException(
_("Only `SELECT` statements are allowed against this database")
raise SupersetErrorException(
SupersetError(
message=__("Only SELECT statements are allowed against this database."),
error_type=SupersetErrorType.DML_NOT_ALLOWED_ERROR,
level=ErrorLevel.ERROR,
)
)
if apply_ctas:
if not query.tmp_table_name:
@ -353,7 +368,13 @@ def execute_sql_statements( # pylint: disable=too-many-arguments, too-many-loca
db_engine_spec.patch()
if database.allow_run_async and not results_backend:
raise SqlLabException("Results backend isn't configured.")
raise SupersetErrorException(
SupersetError(
message=__("Results backend is not configured."),
error_type=SupersetErrorType.RESULTS_BACKEND_NOT_CONFIGURED_ERROR,
level=ErrorLevel.ERROR,
)
)
# Breaking down into multiple statements
parsed_query = ParsedQuery(rendered_query, strip_comments=True)
@ -377,11 +398,16 @@ def execute_sql_statements( # pylint: disable=too-many-arguments, too-many-loca
and query.ctas_method == CtasMethod.TABLE
and not parsed_query.is_valid_ctas()
):
raise SqlLabException(
_(
"CTAS (create table as select) can only be run with a query where "
"the last statement is a SELECT. Please make sure your query has "
"a SELECT as its last statement. Then, try running your query again."
raise SupersetErrorException(
SupersetError(
message=__(
"CTAS (create table as select) can only be run with a query where "
"the last statement is a SELECT. Please make sure your query has "
"a SELECT as its last statement. Then, try running your query "
"again."
),
error_type=SupersetErrorType.INVALID_CTAS_QUERY_ERROR,
level=ErrorLevel.ERROR,
)
)
if (
@ -389,11 +415,15 @@ def execute_sql_statements( # pylint: disable=too-many-arguments, too-many-loca
and query.ctas_method == CtasMethod.VIEW
and not parsed_query.is_valid_cvas()
):
raise SqlLabException(
_(
"CVAS (create view as select) can only be run with a query with "
"a single SELECT statement. Please make sure your query has only "
"a SELECT statement. Then, try running your query again."
raise SupersetErrorException(
SupersetError(
message=__(
"CVAS (create view as select) can only be run with a query with "
"a single SELECT statement. Please make sure your query has only "
"a SELECT statement. Then, try running your query again."
),
error_type=SupersetErrorType.INVALID_CVAS_QUERY_ERROR,
level=ErrorLevel.ERROR,
)
)
@ -438,9 +468,14 @@ def execute_sql_statements( # pylint: disable=too-many-arguments, too-many-loca
)
except Exception as ex: # pylint: disable=broad-except
msg = str(ex)
if statement_count > 1:
msg = f"[Statement {i+1} out of {statement_count}] " + msg
payload = handle_query_error(msg, query, session, payload)
prefix_message = (
f"[Statement {i+1} out of {statement_count}]"
if statement_count > 1
else ""
)
payload = handle_query_error(
ex, query, session, payload, prefix_message
)
return payload
# Commit the connection so CTA queries will create the table.

12
superset/views/core.py
View File

@ -73,12 +73,13 @@ from superset.dashboards.dao import DashboardDAO
from superset.databases.dao import DatabaseDAO
from superset.databases.filters import DatabaseFilter
from superset.datasets.commands.exceptions import DatasetNotFoundError
from superset.errors import SupersetError
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import (
CacheLoadError,
CertificateException,
DatabaseNotFound,
SerializationError,
SupersetErrorException,
SupersetErrorsException,
SupersetException,
SupersetGenericDBErrorException,
@ -2179,7 +2180,13 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
of rows returned.
"""
if not results_backend:
return json_error_response("Results backend isn't configured")
raise SupersetErrorException(
SupersetError(
message=__("Results backend is not configured."),
error_type=SupersetErrorType.RESULTS_BACKEND_NOT_CONFIGURED_ERROR,
level=ErrorLevel.ERROR,
)
)
read_from_results_backend_start = now_as_float()
blob = results_backend.get(key)
@ -2460,7 +2467,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
raise SupersetErrorsException(
[SupersetError(**params) for params in data["errors"]]
)
# old string-only error message
raise SupersetGenericDBErrorException(data["error"])

120
tests/sqllab_tests.py
View File

@ -29,7 +29,8 @@ import prison
from superset import db, security_manager
from superset.connectors.sqla.models import SqlaTable
from superset.db_engine_specs import BaseEngineSpec
from superset.errors import ErrorLevel, SupersetErrorType
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetErrorException
from superset.models.core import Database
from superset.models.sql_lab import LimitingFactor, Query, SavedQuery
from superset.result_set import SupersetResultSet
@ -120,6 +121,29 @@ class TestSqlLab(SupersetTestCase):
"engine_name": engine_name,
}
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_sql_json_dml_disallowed(self):
self.login("admin")
data = self.run_sql("DELETE FROM birth_names", "1")
assert data == {
"errors": [
{
"message": "Only SELECT statements are allowed against this database.",
"error_type": SupersetErrorType.DML_NOT_ALLOWED_ERROR,
"level": ErrorLevel.ERROR,
"extra": {
"issue_codes": [
{
"code": 1022,
"message": "Issue 1022 - Database does not allow data manipulation.",
}
]
},
}
]
}
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_sql_json_to_saved_query_info(self):
"""
@ -744,6 +768,58 @@ class TestSqlLab(SupersetTestCase):
]
)
@mock.patch("superset.sql_lab.results_backend", None)
@mock.patch("superset.sql_lab.get_query")
@mock.patch("superset.sql_lab.execute_sql_statement")
def test_execute_sql_statements_no_results_backend(
self, mock_execute_sql_statement, mock_get_query
):
sql = """
-- comment
SET @value = 42;
SELECT @value AS foo;
-- comment
"""
mock_session = mock.MagicMock()
mock_query = mock.MagicMock()
mock_query.database.allow_run_async = True
mock_cursor = mock.MagicMock()
mock_query.database.get_sqla_engine.return_value.raw_connection.return_value.cursor.return_value = (
mock_cursor
)
mock_query.database.db_engine_spec.run_multiple_statements_as_one = False
mock_get_query.return_value = mock_query
with pytest.raises(SupersetErrorException) as excinfo:
execute_sql_statements(
query_id=1,
rendered_query=sql,
return_results=True,
store_results=False,
user_name="admin",
session=mock_session,
start_time=None,
expand_data=False,
log_params=None,
)
assert excinfo.value.error == SupersetError(
message="Results backend is not configured.",
error_type=SupersetErrorType.RESULTS_BACKEND_NOT_CONFIGURED_ERROR,
level=ErrorLevel.ERROR,
extra={
"issue_codes": [
{
"code": 1021,
"message": (
"Issue 1021 - Results backend needed for asynchronous "
"queries is not configured."
),
}
]
},
)
@mock.patch("superset.sql_lab.get_query")
@mock.patch("superset.sql_lab.execute_sql_statement")
def test_execute_sql_statements_ctas(
@ -805,7 +881,7 @@ class TestSqlLab(SupersetTestCase):
# try invalid CTAS
sql = "DROP TABLE my_table"
with pytest.raises(SqlLabException) as excinfo:
with pytest.raises(SupersetErrorException) as excinfo:
execute_sql_statements(
query_id=1,
rendered_query=sql,
@ -817,11 +893,18 @@ class TestSqlLab(SupersetTestCase):
expand_data=False,
log_params=None,
)
assert str(excinfo.value) == (
"CTAS (create table as select) can only be run with "
"a query where the last statement is a SELECT. Please "
"make sure your query has a SELECT as its last "
"statement. Then, try running your query again."
assert excinfo.value.error == SupersetError(
message="CTAS (create table as select) can only be run with a query where the last statement is a SELECT. Please make sure your query has a SELECT as its last statement. Then, try running your query again.",
error_type=SupersetErrorType.INVALID_CTAS_QUERY_ERROR,
level=ErrorLevel.ERROR,
extra={
"issue_codes": [
{
"code": 1023,
"message": "Issue 1023 - The CTAS (create table as select) doesn't have a SELECT statement at the end. Please make sure your query has a SELECT as its last statement. Then, try running your query again.",
}
]
},
)
# try invalid CVAS
@ -832,7 +915,7 @@ class TestSqlLab(SupersetTestCase):
SELECT @value AS foo;
-- comment
"""
with pytest.raises(SqlLabException) as excinfo:
with pytest.raises(SupersetErrorException) as excinfo:
execute_sql_statements(
query_id=1,
rendered_query=sql,
@ -844,11 +927,22 @@ class TestSqlLab(SupersetTestCase):
expand_data=False,
log_params=None,
)
assert str(excinfo.value) == (
"CVAS (create view as select) can only be run with a "
"query with a single SELECT statement. Please make "
"sure your query has only a SELECT statement. Then, "
"try running your query again."
assert excinfo.value.error == SupersetError(
message="CVAS (create view as select) can only be run with a query with a single SELECT statement. Please make sure your query has only a SELECT statement. Then, try running your query again.",
error_type=SupersetErrorType.INVALID_CVAS_QUERY_ERROR,
level=ErrorLevel.ERROR,
extra={
"issue_codes": [
{
"code": 1024,
"message": "Issue 1024 - CVAS (create view as select) query has more than one statement.",
},
{
"code": 1025,
"message": "Issue 1025 - CVAS (create view as select) query is not a SELECT statement.",
},
]
},
)
@mock.patch("superset.sql_lab.get_query")