[explore] show the broken query when failing (#1871)

* Return query when failing

* Linting

* sjson -> simplejson

superset/assets/javascripts/explorev2/actions/exploreActions.js

@@ -129,8 +129,8 @@ export function chartUpdateSucceeded(query) {
 }
 
 export const CHART_UPDATE_FAILED = 'CHART_UPDATE_FAILED';
-export function chartUpdateFailed(error) {
-  return { type: CHART_UPDATE_FAILED, error };
+export function chartUpdateFailed(error, query) {
+  return { type: CHART_UPDATE_FAILED, error, query };
 }
 
 export const UPDATE_EXPLORE_ENDPOINTS = 'UPDATE_EXPLORE_ENDPOINTS';

superset/assets/javascripts/explorev2/components/ChartContainer.jsx

@@ -142,7 +142,13 @@ class ChartContainer extends React.Component {
       },
 
       error(msg) {
-        props.actions.chartUpdateFailed(msg);
+        let payload = { error: msg };
+        try {
+          payload = JSON.parse(msg);
+        } catch (e) {
+          // pass
+        }
+        props.actions.chartUpdateFailed(payload.error, payload.query);
       },
 
       d3format: (col, number) => {

superset/assets/javascripts/explorev2/reducers/exploreReducer.js

@@ -108,16 +108,14 @@ export const exploreReducer = function (state, action) {
       );
     },
     [actions.CHART_UPDATE_SUCCEEDED]() {
-      const vizUpdates = {
-        query: action.query,
-      };
       return Object.assign(
         {},
         state,
         {
           chartStatus: 'success',
-          viz: Object.assign({}, state.viz, vizUpdates),
-        });
+          viz: Object.assign({}, state.viz, { query: action.query }),
+        }
+      );
     },
     [actions.CHART_UPDATE_STARTED]() {
       const chartUpdateStartTime = now();
@@ -138,9 +136,12 @@ export const exploreReducer = function (state, action) {
         });
     },
     [actions.CHART_UPDATE_FAILED]() {
-      const chartUpdateEndTime = now();
-      return Object.assign({}, state,
-        { chartStatus: 'failed', chartAlert: action.error, chartUpdateEndTime });
+      return Object.assign({}, state, {
+        chartStatus: 'failed',
+        chartAlert: action.error,
+        chartUpdateEndTime: now(),
+        viz: Object.assign({}, state.viz, { query: action.query }),
+      });
     },
     [actions.UPDATE_CHART_STATUS]() {
       const newState = Object.assign({}, state, { chartStatus: action.status });

superset/assets/javascripts/modules/superset.js

@@ -68,11 +68,6 @@ const px = function () {
       $('#timer').text(num.toFixed(2) + ' sec');
     };
     let qrystr = '';
-    const always = function () {
-      // Private f, runs after done and error
-      clearInterval(timer);
-      $('#timer').removeClass('btn-warning');
-    };
     slice = {
       data,
       container,
@@ -123,6 +118,13 @@ const px = function () {
         return utils.d3format(format, number);
       },
       /* eslint no-shadow: 0 */
+      always(data) {
+        clearInterval(timer);
+        $('#timer').removeClass('btn-warning');
+        if (data && data.query) {
+          slice.viewSqlQuery = data.query;
+        }
+      },
       done(payload) {
         Object.assign(data, payload);
 
@@ -131,14 +133,10 @@ const px = function () {
         container.fadeTo(0.5, 1);
         container.show();
 
-        if (data !== undefined) {
-          slice.viewSqlQuery = data.query;
-        }
-
-        $('#timer').removeClass('label-warning label-danger');
         $('#timer').addClass('label-success');
+        $('#timer').removeClass('label-warning label-danger');
         $('.query-and-save button').removeAttr('disabled');
-        always(data);
+        this.always(data);
         controller.done(this);
       },
       getErrorMsg(xhr) {
@@ -161,8 +159,9 @@ const px = function () {
         token.find('img.loading').hide();
         container.fadeTo(0.5, 1);
         let errHtml = '';
+        let o;
         try {
-          const o = JSON.parse(msg);
+          o = JSON.parse(msg);
           if (o.error) {
             errorMsg = o.error;
           }
@@ -181,7 +180,7 @@ const px = function () {
         $('span.query').removeClass('disabled');
         $('#timer').addClass('btn-danger');
         $('.query-and-save button').removeAttr('disabled');
-        always(data);
+        this.always(o);
         controller.error(this);
       },
       clearError() {

superset/models.py

@@ -11,7 +11,6 @@
 import pickle
 import re
 import textwrap
-from collections import namedtuple
 from copy import deepcopy, copy
 from datetime import timedelta, datetime, date
 
@@ -59,13 +58,30 @@
 from superset.jinja_context import get_template_processor
 from superset.utils import (
     flasher, MetricPermException, DimSelector, wrap_clause_in_parens,
-    DTTM_ALIAS,
+    DTTM_ALIAS, QueryStatus,
 )
 
-
 config = app.config
 
-QueryResult = namedtuple('namedtuple', ['df', 'query', 'duration'])
+
+class QueryResult(object):
+
+    """Object returned by the query interface"""
+
+    def __init__(  # noqa
+            self,
+            df,
+            query,
+            duration,
+            status=QueryStatus.SUCCESS,
+            error_message=None):
+        self.df = df
+        self.query = query
+        self.duration = duration
+        self.status = status
+        self.error_message = error_message
+
+
 FillterPattern = re.compile(r'''((?:[^,"']|"[^"]*"|'[^']*')+)''')
 
 
@@ -1195,13 +1211,22 @@ def visit_column(element, compiler, **kw):
             qry.compile(
                 engine, compile_kwargs={"literal_binds": True},),
         )
-        df = pd.read_sql_query(
-            sql=sql,
-            con=engine
-        )
         sql = sqlparse.format(sql, reindent=True)
+        status = QueryStatus.SUCCESS
+        error_message = None
+        df = None
+        try:
+            df = pd.read_sql_query(sql, con=engine)
+        except Exception as e:
+            status = QueryStatus.FAILED
+            error_message = str(e)
+
         return QueryResult(
-            df=df, duration=datetime.now() - qry_start_dttm, query=sql)
+            status=status,
+            df=df,
+            duration=datetime.now() - qry_start_dttm,
+            query=sql,
+            error_message=error_message)
 
     def get_sqla_table_object(self):
         return self.database.get_table(self.table_name, schema=self.schema)
@@ -2548,16 +2573,6 @@ class FavStar(Model):
     dttm = Column(DateTime, default=datetime.utcnow)
 
 
-class QueryStatus:
-    CANCELLED = 'cancelled'
-    FAILED = 'failed'
-    PENDING = 'pending'
-    RUNNING = 'running'
-    SCHEDULED = 'scheduled'
-    SUCCESS = 'success'
-    TIMED_OUT = 'timed_out'
-
-
 class Query(Model):
 
     """ORM model for SQL query"""

superset/utils.py

@@ -405,3 +405,16 @@ def ping_connection(dbapi_connection, connection_record, connection_proxy):
         except:
             raise exc.DisconnectionError()
         cursor.close()
+
+
+class QueryStatus:
+
+    """Enum-type class for query statuses"""
+
+    CANCELLED = 'cancelled'
+    FAILED = 'failed'
+    PENDING = 'pending'
+    RUNNING = 'running'
+    SCHEDULED = 'scheduled'
+    SUCCESS = 'success'
+    TIMED_OUT = 'timed_out'

superset/views.py

@@ -5,6 +5,7 @@
 
 from datetime import datetime, timedelta
 import json
+import simplejson
 import logging
 import pickle
 import re
@@ -1390,16 +1391,22 @@ def explore_json(self, datasource_type, datasource_id):
                 status=404,
                 mimetype="application/json")
 
-        payload = ""
+        payload = {}
+        status = 200
         try:
-            payload = viz_obj.get_json()
+            payload = viz_obj.get_payload()
         except Exception as e:
             logging.exception(e)
+            status = 500
             return json_error_response(utils.error_msg_from_exception(e))
 
+        if payload.get('status') == QueryStatus.FAILED:
+            status = 500
+
         return Response(
-            payload,
-            status=200,
+            simplejson.dumps(
+                payload, default=utils.json_int_dttm_ser, ignore_nan=True),
+            status=status,
             mimetype="application/json")
 
     @expose("/import_dashboards", methods=['GET', 'POST'])
@@ -2261,13 +2268,13 @@ def sqllab_viz(self):
         db.session.commit()
         params = {
             'viz_type': viz_type,
-            'groupby': dims[0].column_name if dims else '',
-            'metrics': metrics[0].metric_name if metrics else '',
-            'metric': metrics[0].metric_name if metrics else '',
+            'groupby': dims[0].column_name if dims else None,
+            'metrics': metrics[0].metric_name if metrics else None,
+            'metric': metrics[0].metric_name if metrics else None,
             'since': '100 years ago',
             'limit': '0',
         }
-        params = "&".join([k + '=' + v for k, v in params.items()])
+        params = "&".join([k + '=' + v for k, v in params.items() if v])
         url = '/superset/explore/table/{table.id}/?{params}'.format(**locals())
         return redirect(url)