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

feat: show missing parameters in query (#12049) 

* feat: show missing parameters in query

* Fix lint

* Address comments

* Simplify error message

* Use f-string in helper function
This commit is contained in:
Beto Dealmeida 2020-12-15 18:47:40 -08:00 committed by GitHub
parent 8da1900d8a
commit 8bda6b0bd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 108 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -73,3 +73,14 @@ The table was deleted or renamed in the database.
Your query failed because it is referencing a table that no longer exists in
the underlying database. You should modify your query to reference the correct
table.
## Issue 1006
```
One or more parameters specified in the query are missing.
```
Your query was not submitted to the database because it's missing one or more
parameters. You should define all the parameters referenced in the query in a
valid JSON document. Check that the parameters are spelled correctly and that
the document has a valid syntax.

10
superset-frontend/src/SqlLab/components/ResultSet.tsx
View File

@ -28,6 +28,7 @@ import { styled, t } from '@superset-ui/core';
import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
import { getByUser, put as updateDatset } from 'src/api/dataset';
import { ErrorTypeEnum } from 'src/components/ErrorMessage/types';
import Loading from '../../components/Loading';
import ExploreCtasResultsButton from './ExploreCtasResultsButton';
import ExploreResultsButton from './ExploreResultsButton';
@ -489,10 +490,17 @@ export default class ResultSet extends React.PureComponent<
return <Alert bsStyle="warning">Query was stopped</Alert>;
}
if (query.state === 'failed') {
// TODO (betodealmeida): handle this logic through the error component
// registry
const title =
query?.errors?.[0].error_type ===
ErrorTypeEnum.MISSING_TEMPLATE_PARAMS_ERROR
? t('Parameter Error')
: t('Database Error');
return (
<div className="result-set-error-message">
<ErrorMessageWithStackTrace
title={t('Database Error')}
title={title}
error={query?.errors?.[0]}
subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
copyText={query.errorMessage || undefined}

16
superset-frontend/src/SqlLab/components/SqlEditor.jsx
View File

@ -566,13 +566,15 @@ class SqlEditor extends React.PureComponent {
<Checkbox checked={this.state.autocompleteEnabled} />{' '}
{t('Autocomplete')}
</Button>{' '}
<TemplateParamsEditor
language="json"
onChange={params => {
this.props.actions.queryEditorSetTemplateParams(qe, params);
}}
code={qe.templateParams}
/>
{isFeatureEnabled(FeatureFlag.ENABLE_TEMPLATE_PROCESSING) && (
<TemplateParamsEditor
language="json"
onChange={params => {
this.props.actions.queryEditorSetTemplateParams(qe, params);
}}
code={qe.templateParams}
/>
)}
{limitWarning}
{this.props.latestQuery && (
<Timer

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

@ -42,6 +42,9 @@ export const ErrorTypeEnum = {
// Other errors
BACKEND_TIMEOUT_ERROR: 'BACKEND_TIMEOUT_ERROR',
// Sqllab error
MISSING_TEMPLATE_PARAMS_ERROR: 'MISSING_TEMPLATE_PARAMS_ERROR',
} as const;
type ValueOf<T> = T[keyof T];

1
superset-frontend/src/featureFlags.ts
View File

@ -35,6 +35,7 @@ export enum FeatureFlag {
ESCAPE_MARKDOWN_HTML = 'ESCAPE_MARKDOWN_HTML',
VERSIONED_EXPORT = 'VERSIONED_EXPORT',
GLOBAL_ASYNC_QUERIES = 'GLOBAL_ASYNC_QUERIES',
ENABLE_TEMPLATE_PROCESSING = 'ENABLE_TEMPLATE_PROCESSING',
}
export type FeatureFlagMap = {

13
superset/errors.py
View File

@ -27,6 +27,7 @@ class SupersetErrorType(str, Enum):
Types of errors that can exist within Superset.
Keep in sync with superset-frontend/src/components/ErrorMessage/types.ts
and docs/src/pages/docs/Miscellaneous/issue_codes.mdx
"""
# Frontend errors
@ -52,6 +53,9 @@ class SupersetErrorType(str, Enum):
# Other errors
BACKEND_TIMEOUT_ERROR = "BACKEND_TIMEOUT_ERROR"
# Sql Lab errors
MISSING_TEMPLATE_PARAMS_ERROR = "MISSING_TEMPLATE_PARAMS_ERROR"
ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
SupersetErrorType.BACKEND_TIMEOUT_ERROR: [
@ -100,6 +104,15 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
),
},
],
SupersetErrorType.MISSING_TEMPLATE_PARAMS_ERROR: [
{
"code": 1006,
"message": _(
"Issue 1006 - One or more parameters specified in the query are "
"missing."
),
},
],
}

5
superset/jinja_context.py
View File

@ -22,6 +22,7 @@ from typing import Any, Callable, cast, Dict, List, Optional, Tuple, TYPE_CHECKI
from flask import current_app, g, request
from flask_babel import gettext as _
from jinja2 import DebugUndefined
from jinja2.sandbox import SandboxedEnvironment
from superset.exceptions import SupersetTemplateException
@ -60,7 +61,7 @@ def context_addons() -> Dict[str, Any]:
def filter_values(column: str, default: Optional[str] = None) -> List[str]:
""" Gets a values for a particular filter as a list
"""Gets a values for a particular filter as a list
This is useful if:
- you want to use a filter box to filter a query where the name of filter box
@ -296,7 +297,7 @@ class BaseTemplateProcessor: # pylint: disable=too-few-public-methods
self._schema = table.schema
self._extra_cache_keys = extra_cache_keys
self._context: Dict[str, Any] = {}
self._env = SandboxedEnvironment()
self._env = SandboxedEnvironment(undefined=DebugUndefined)
self.set_context(**kwargs)
def set_context(self, **kwargs: Any) -> None:

5
superset/utils/core.py
View File

@ -1669,3 +1669,8 @@ def get_time_filter_status( # pylint: disable=too-many-branches
)
return applied, rejected
def format_list(items: Sequence[str], sep: str = ", ", quote: str = '"') -> str:
quote_escaped = "\\" + quote
return sep.join(f"{quote}{x.replace(quote, quote_escaped)}{quote}" for x in items)

34
superset/views/core.py
View File

@ -14,7 +14,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# pylint: disable=comparison-with-callable
# pylint: disable=comparison-with-callable, line-too-long, too-many-branches
import dataclasses
import logging
import re
from contextlib import closing
@ -34,8 +35,9 @@ from flask_appbuilder.security.decorators import (
permission_name,
)
from flask_appbuilder.security.sqla import models as ab_models
from flask_babel import gettext as __, lazy_gettext as _
from flask_babel import gettext as __, lazy_gettext as _, ngettext
from jinja2.exceptions import TemplateError
from jinja2.meta import find_undeclared_variables
from sqlalchemy import and_, or_
from sqlalchemy.engine.url import make_url
from sqlalchemy.exc import ArgumentError, DBAPIError, NoSuchModuleError, SQLAlchemyError
@ -69,6 +71,7 @@ from superset.dashboards.commands.importers.v0 import ImportDashboardsCommand
from superset.dashboards.dao import DashboardDAO
from superset.databases.dao import DatabaseDAO
from superset.databases.filters import DatabaseFilter
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import (
CacheLoadError,
CertificateException,
@ -157,6 +160,11 @@ DATABASE_KEYS = [
DATASOURCE_MISSING_ERR = __("The data source seems to have been deleted")
USER_MISSING_ERR = __("The user seems to have been deleted")
PARAMETER_MISSING_ERR = (
"Please check your template parameters for syntax errors and make sure "
"they match across your SQL query and Set Parameters. Then, try running "
"your query again."
)
class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
@ -2508,6 +2516,28 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
f"Query {query_id}: Template syntax error: {error_msg}"
)
# pylint: disable=protected-access
if is_feature_enabled("ENABLE_TEMPLATE_PROCESSING"):
ast = template_processor._env.parse(rendered_query)
undefined = find_undeclared_variables(ast) # type: ignore
if undefined:
error = SupersetError(
message=ngettext(
"There's an error with the parameter %(parameters)s.",
"There's an error with the parameters %(parameters)s.",
len(undefined),
parameters=utils.format_list(undefined),
)
+ " "
+ PARAMETER_MISSING_ERR,
level=ErrorLevel.ERROR,
error_type=SupersetErrorType.MISSING_TEMPLATE_PARAMS_ERROR,
extra={"missing_parameters": list(undefined)},
)
return json_error_response(
payload={"errors": [dataclasses.asdict(error)]},
)
# Limit is not applied to the CTA queries if SQLLAB_CTAS_NO_LIMIT flag is set
# to True.
if not (config.get("SQLLAB_CTAS_NO_LIMIT") and select_as_cta):

2
tests/base_tests.py
View File

@ -324,6 +324,7 @@ class SupersetTestCase(TestCase):
tmp_table_name=None,
schema=None,
ctas_method=CtasMethod.TABLE,
template_params="{}",
):
if user_name:
self.logout()
@ -336,6 +337,7 @@ class SupersetTestCase(TestCase):
"queryLimit": query_limit,
"sql_editor_id": sql_editor_id,
"ctas_method": ctas_method,
"templateParams": template_params,
}
if tmp_table_name:
json_payload["tmp_table_name"] = tmp_table_name

21
tests/sqllab_tests.py
View File

@ -552,7 +552,6 @@ class TestSqlLab(SupersetTestCase):
url = "/api/v1/query/"
data = self.get_json_resp(url)
admin = security_manager.find_user("admin")
self.assertEqual(3, len(data["result"]))
def test_api_database(self):
@ -576,3 +575,23 @@ class TestSqlLab(SupersetTestCase):
{r.get("database_name") for r in self.get_json_resp(url)["result"]},
)
self.delete_fake_db()
@mock.patch.dict(
"superset.extensions.feature_flag_manager._feature_flags",
{"ENABLE_TEMPLATE_PROCESSING": True},
clear=True,
)
def test_sql_json_parameter_error(self):
self.login("admin")
data = self.run_sql(
"SELECT * FROM birth_names WHERE state = '{{ state }}' LIMIT 10",
"1",
template_params=json.dumps({"state": "CA"}),
)
assert data["status"] == "success"
data = self.run_sql(
"SELECT * FROM birth_names WHERE state = '{{ state }}' LIMIT 10", "2"
)
assert data["errors"][0]["error_type"] == "MISSING_TEMPLATE_PARAMS_ERROR"