mapledan commented on code in PR #42785:
URL: https://github.com/apache/superset/pull/42785#discussion_r4036209806
##########
superset/commands/sql_lab/estimate.py:
##########
@@ -161,27 +162,51 @@ def run(
) -> list[dict[str, Any]]:
self.validate()
- sql = self._sql
- if self._template_params:
- # Access is already checked in validate() before any rendering.
- template_processor = get_template_processor(self._database)
- try:
- sql = template_processor.process_template(sql,
**self._template_params)
- except TemplateError as ex:
- raise SupersetErrorException(
- SupersetError(
- message=str(ex),
- error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
- level=ErrorLevel.ERROR,
- ),
- status=400,
- ) from ex
+ # Access is already checked in validate() before any rendering.
+ #
+ # Rendered whether or not `template_params` was supplied, the way
+ # `validate()` above already jinja-processes for authorization and the
+ # execution path does in `SqlQueryRenderImpl.render`. A query needs no
+ # declared parameter to need rendering -- `get_time_filter()`,
+ # `current_username()`, `url_param()` take none -- and SQL Lab posts an
+ # empty `template_params` for an estimate, so those never rendered.
+ template_processor = get_template_processor(self._database)
+ try:
+ sql = template_processor.process_template(
+ self._sql, **self._template_params
+ )
+ except TemplateError as ex:
+ raise SupersetErrorException(
+ SupersetError(
+ message=str(ex),
+ error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
+ level=ErrorLevel.ERROR,
+ ),
+ status=400,
+ ) from ex
# Apply the same SQL security controls used by the execution path
# (sql_lab.execute_sql_statements) so cost estimation cannot be used to
# probe disallowed functions/tables, bypass the DML guard, or confirm
# the existence of rows hidden by row-level security.
- sql = self._apply_sql_security(sql)
+ try:
+ sql = self._apply_sql_security(sql)
+ except SupersetParseError as ex:
Review Comment:
Gone with the block. A missing parameter is caught before parsing, so there
is
nothing left to rewrite and a `SupersetParseError` from `_apply_sql_security`
propagates as itself — RLS and length errors included. Covered by a test that
malformed SQL keeps the parser's own error.
##########
superset/commands/sql_lab/estimate.py:
##########
@@ -161,27 +162,51 @@ def run(
) -> list[dict[str, Any]]:
self.validate()
- sql = self._sql
- if self._template_params:
- # Access is already checked in validate() before any rendering.
- template_processor = get_template_processor(self._database)
- try:
- sql = template_processor.process_template(sql,
**self._template_params)
- except TemplateError as ex:
- raise SupersetErrorException(
- SupersetError(
- message=str(ex),
- error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
- level=ErrorLevel.ERROR,
- ),
- status=400,
- ) from ex
+ # Access is already checked in validate() before any rendering.
+ #
+ # Rendered whether or not `template_params` was supplied, the way
+ # `validate()` above already jinja-processes for authorization and the
+ # execution path does in `SqlQueryRenderImpl.render`. A query needs no
+ # declared parameter to need rendering -- `get_time_filter()`,
+ # `current_username()`, `url_param()` take none -- and SQL Lab posts an
+ # empty `template_params` for an estimate, so those never rendered.
+ template_processor = get_template_processor(self._database)
Review Comment:
Gone with the block. A missing parameter is caught before parsing, so there
is
nothing left to rewrite and a `SupersetParseError` from `_apply_sql_security`
propagates as itself — RLS and length errors included. Covered by a test that
malformed SQL keeps the parser's own error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
