From b88ca181ad1e23cd3f356ba1e7f998488ca0dcff Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Mon, 26 Mar 2018 19:42:33 +0300 Subject: [PATCH 1/2] Preprocess SQL Lab query prior to checking syntax (#4686) Syntax checking doesn't work if jinja templates haven't been prerendered. Also remove unreachable return statement. Fixes #4288. (cherry picked from commit 4ec82582c6c6e750a7bbd39b56a58fbaa81349c2) --- superset/sql_lab.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/superset/sql_lab.py b/superset/sql_lab.py index 7b74ab72184af..b752e4d7e9c89 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -161,8 +161,18 @@ def handle_error(msg): if store_results and not results_backend: return handle_error("Results backend isn't configured.") + try: + template_processor = get_template_processor( + database=database, query=query) + tp = template_params or {} + rendered_query = template_processor.process_template(query.sql, **tp) + except Exception as e: + logging.exception(e) + msg = 'Template rendering failed: ' + utils.error_msg_from_exception(e) + return handle_error(msg) + # Limit enforced only for retrieving the data, not for the CTA queries. - superset_query = SupersetQuery(query.sql) + superset_query = SupersetQuery(rendered_query) executed_sql = superset_query.stripped() if not superset_query.is_select() and not database.allow_dml: return handle_error( @@ -172,7 +182,6 @@ def handle_error(msg): return handle_error( 'Only `SELECT` statements can be used with the CREATE TABLE ' 'feature.') - return if not query.tmp_table_name: start_dttm = datetime.fromtimestamp(query.start_time) query.tmp_table_name = 'tmp_{}_table_{}'.format( @@ -183,16 +192,6 @@ def handle_error(msg): db_engine_spec.limit_method == LimitMethod.WRAP_SQL): executed_sql = database.wrap_sql_limit(executed_sql, query.limit) query.limit_used = True - try: - template_processor = get_template_processor( - database=database, query=query) - tp = template_params or {} - executed_sql = template_processor.process_template( - executed_sql, **tp) - except Exception as e: - logging.exception(e) - msg = 'Template rendering failed: ' + utils.error_msg_from_exception(e) - return handle_error(msg) query.executed_sql = executed_sql query.status = QueryStatus.RUNNING From 75a2bdfe03e57d5987af823cc6aee14cc445f31b Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Thu, 29 Mar 2018 16:11:23 -0700 Subject: [PATCH 2/2] [sqllab] Using app context for Celery task (#4669) (cherry picked from commit d49a0e7958ea5a7822d125b1ededffcb1563656c) --- superset/sql_lab.py | 22 +++++----------------- superset/views/core.py | 23 ++++++++++++++++++----- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/superset/sql_lab.py b/superset/sql_lab.py index b752e4d7e9c89..a3239eefca0db 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -19,7 +19,6 @@ from superset import app, dataframe, db, results_backend, utils from superset.db_engine_specs import LimitMethod -from superset.jinja_context import get_template_processor from superset.models.sql_lab import Query from superset.sql_parse import SupersetQuery from superset.utils import get_celery_app, QueryStatus @@ -109,13 +108,12 @@ def convert_results_to_df(cursor_description, data): @celery_app.task(bind=True, soft_time_limit=SQLLAB_TIMEOUT) def get_sql_results( - ctask, query_id, return_results=True, store_results=False, - user_name=None, template_params=None): + ctask, query_id, rendered_query, return_results=True, store_results=False, + user_name=None): """Executes the sql query returns the results.""" try: return execute_sql( - ctask, query_id, return_results, store_results, user_name, - template_params) + ctask, query_id, rendered_query, return_results, store_results, user_name) except Exception as e: logging.exception(e) stats_logger.incr('error_sqllab_unhandled') @@ -129,8 +127,8 @@ def get_sql_results( def execute_sql( - ctask, query_id, return_results=True, store_results=False, user_name=None, - template_params=None, + ctask, query_id, rendered_query, return_results=True, store_results=False, + user_name=None, ): """Executes the sql query returns the results.""" session = get_session(not ctask.request.called_directly) @@ -161,16 +159,6 @@ def handle_error(msg): if store_results and not results_backend: return handle_error("Results backend isn't configured.") - try: - template_processor = get_template_processor( - database=database, query=query) - tp = template_params or {} - rendered_query = template_processor.process_template(query.sql, **tp) - except Exception as e: - logging.exception(e) - msg = 'Template rendering failed: ' + utils.error_msg_from_exception(e) - return handle_error(msg) - # Limit enforced only for retrieving the data, not for the CTA queries. superset_query = SupersetQuery(rendered_query) executed_sql = superset_query.stripped() diff --git a/superset/views/core.py b/superset/views/core.py index d56d5ff6f5198..b800b5e126e0e 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -39,6 +39,7 @@ from superset.connectors.connector_registry import ConnectorRegistry from superset.connectors.sqla.models import AnnotationDatasource, SqlaTable from superset.forms import CsvToDatabaseForm +from superset.jinja_context import get_template_processor from superset.legacy import cast_form_data import superset.models.core as models from superset.models.sql_lab import Query @@ -2403,16 +2404,27 @@ def sql_json(self): raise Exception(_('Query record was not created as expected.')) logging.info('Triggering query_id: {}'.format(query_id)) + try: + template_processor = get_template_processor( + database=query.database, query=query) + rendered_query = template_processor.process_template( + query.sql, + **template_params) + except Exception as e: + return json_error_response( + 'Template rendering failed: {}'.format(utils.error_msg_from_exception(e))) + # Async request. if async: logging.info('Running query on a Celery worker') # Ignore the celery future object and the request may time out. try: sql_lab.get_sql_results.delay( - query_id=query_id, return_results=False, + query_id, + rendered_query, + return_results=False, store_results=not query.select_as_cta, - user_name=g.user.username, - template_params=template_params) + user_name=g.user.username) except Exception as e: logging.exception(e) msg = ( @@ -2441,8 +2453,9 @@ def sql_json(self): error_message=timeout_msg): # pylint: disable=no-value-for-parameter data = sql_lab.get_sql_results( - query_id=query_id, return_results=True, - template_params=template_params) + query_id, + rendered_query, + return_results=True) payload = json.dumps( data, default=utils.pessimistic_json_iso_dttm_ser) except Exception as e: