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

docs/src/pages/docs/Miscellaneous/issue_codes.mdx

@@ -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.

superset-frontend/src/SqlLab/components/ResultSet.tsx

@@ -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}

superset-frontend/src/SqlLab/components/SqlEditor.jsx

@@ -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

superset-frontend/src/components/ErrorMessage/types.ts

@@ -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];

superset-frontend/src/featureFlags.ts

@@ -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 = {

superset/errors.py

@@ -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 @@ class SupersetErrorType(str, Enum):
             ),
         },
     ],
+    SupersetErrorType.MISSING_TEMPLATE_PARAMS_ERROR: [
+        {
+            "code": 1006,
+            "message": _(
+                "Issue 1006 - One or more parameters specified in the query are "
+                "missing."
+            ),
+        },
+    ],
 }
 
 

superset/jinja_context.py

@@ -22,6 +22,7 @@
 
 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 @@ def __init__(
             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:

superset/utils/core.py

@@ -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)

superset/views/core.py

@@ -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 @@
     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.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 @@
 
 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 @@ def sql_json_exec(  # pylint: disable=too-many-statements,too-many-locals
                 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):

tests/base_tests.py

@@ -324,6 +324,7 @@ def run_sql(
         tmp_table_name=None,
         schema=None,
         ctas_method=CtasMethod.TABLE,
+        template_params="{}",
     ):
         if user_name:
             self.logout()
@@ -336,6 +337,7 @@ def run_sql(
             "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

tests/sqllab_tests.py

@@ -552,7 +552,6 @@ def test_query_admin_can_access_all_queries(self) -> None:
 
         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 @@ def test_api_database(self):
             {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"