Merge pull request #30 from lyft/mergeSQLUIMaster

 feat: Live query validation in the SQL Lab UI

superset/assets/src/SqlLab/actions/sqlLab.js

@@ -65,6 +65,10 @@ export const CLEAR_QUERY_RESULTS = 'CLEAR_QUERY_RESULTS';
 export const REMOVE_DATA_PREVIEW = 'REMOVE_DATA_PREVIEW';
 export const CHANGE_DATA_PREVIEW_ID = 'CHANGE_DATA_PREVIEW_ID';
 
+export const START_QUERY_VALIDATION = 'START_QUERY_VALIDATION';
+export const QUERY_VALIDATION_RETURNED = 'QUERY_VALIDATION_RETURNED';
+export const QUERY_VALIDATION_FAILED = 'QUERY_VALIDATION_FAILED';
+
 export const CREATE_DATASOURCE_STARTED = 'CREATE_DATASOURCE_STARTED';
 export const CREATE_DATASOURCE_SUCCESS = 'CREATE_DATASOURCE_SUCCESS';
 export const CREATE_DATASOURCE_FAILED = 'CREATE_DATASOURCE_FAILED';
@@ -77,6 +81,21 @@ export function resetState() {
   return { type: RESET_STATE };
 }
 
+export function startQueryValidation(query) {
+  Object.assign(query, {
+    id: query.id ? query.id : shortid.generate(),
+  });
+  return { type: START_QUERY_VALIDATION, query };
+}
+
+export function queryValidationReturned(query, results) {
+  return { type: QUERY_VALIDATION_RETURNED, query, results };
+}
+
+export function queryValidationFailed(query, message, error) {
+  return { type: QUERY_VALIDATION_FAILED, query, message, error };
+}
+
 export function saveQuery(query) {
   return dispatch =>
     SupersetClient.post({
@@ -187,6 +206,41 @@ export function runQuery(query) {
   };
 }
 
+export function validateQuery(query) {
+  return function (dispatch) {
+    dispatch(startQueryValidation(query));
+
+    const postPayload = {
+      client_id: query.id,
+      database_id: query.dbId,
+      json: true,
+      schema: query.schema,
+      sql: query.sql,
+      sql_editor_id: query.sqlEditorId,
+      templateParams: query.templateParams,
+      validate_only: true,
+    };
+
+    return SupersetClient.post({
+      endpoint: `/superset/validate_sql_json/${window.location.search}`,
+      postPayload,
+      stringify: false,
+    })
+      .then(({ json }) => {
+        dispatch(queryValidationReturned(query, json));
+      })
+      .catch(response =>
+        getClientErrorObject(response).then((error) => {
+          let message = error.error || error.statusText || t('Unknown error');
+          if (message.includes('CSRF token')) {
+            message = t(COMMON_ERR_MESSAGES.SESSION_TIMED_OUT);
+          }
+          dispatch(queryValidationFailed(query, message, error));
+        }),
+      );
+  };
+}
+
 export function postStopQuery(query) {
   return function (dispatch) {
     return SupersetClient.post({

superset/assets/src/SqlLab/components/AceEditorWrapper.jsx

@@ -157,6 +157,20 @@ class AceEditorWrapper extends React.PureComponent {
       }
     });
   }
+  getAceAnnotations() {
+    const validationResult = this.props.queryEditor.validationResult;
+    const resultIsReady = (validationResult && validationResult.completed);
+    if (resultIsReady && validationResult.errors.length > 0) {
+      const errors = validationResult.errors.map(err => ({
+        type: 'error',
+        row: err.line_number - 1,
+        column: err.start_column - 1,
+        text: err.message,
+      }));
+      return errors;
+    }
+    return [];
+  }
   render() {
     return (
       <AceEditor
@@ -170,6 +184,7 @@ class AceEditorWrapper extends React.PureComponent {
         editorProps={{ $blockScrolling: true }}
         enableLiveAutocompletion
         value={this.state.sql}
+        annotations={this.getAceAnnotations()}
       />
     );
   }

superset/assets/src/SqlLab/components/SqlEditor.jsx

@@ -30,6 +30,7 @@ import {
 } from 'react-bootstrap';
 import Split from 'react-split';
 import { t } from '@superset-ui/translation';
+import debounce from 'lodash/debounce';
 
 import Button from '../../components/Button';
 import LimitControl from './LimitControl';
@@ -52,6 +53,7 @@ const GUTTER_HEIGHT = 5;
 const GUTTER_MARGIN = 3;
 const INITIAL_NORTH_PERCENT = 30;
 const INITIAL_SOUTH_PERCENT = 70;
+const VALIDATION_DEBOUNCE_MS = 600;
 
 const propTypes = {
   actions: PropTypes.object.isRequired,
@@ -88,13 +90,18 @@ class SqlEditor extends React.PureComponent {
     this.elementStyle = this.elementStyle.bind(this);
     this.onResizeStart = this.onResizeStart.bind(this);
     this.onResizeEnd = this.onResizeEnd.bind(this);
+    this.canValidateQuery = this.canValidateQuery.bind(this);
     this.runQuery = this.runQuery.bind(this);
     this.stopQuery = this.stopQuery.bind(this);
     this.onSqlChanged = this.onSqlChanged.bind(this);
     this.setQueryEditorSql = this.setQueryEditorSql.bind(this);
     this.queryPane = this.queryPane.bind(this);
     this.getAceEditorAndSouthPaneHeights = this.getAceEditorAndSouthPaneHeights.bind(this);
     this.getSqlEditorHeight = this.getSqlEditorHeight.bind(this);
+    this.requestValidation = debounce(
+      this.requestValidation.bind(this),
+      VALIDATION_DEBOUNCE_MS,
+    );
   }
   componentWillMount() {
     if (this.state.autorun) {
@@ -126,6 +133,11 @@ class SqlEditor extends React.PureComponent {
   }
   onSqlChanged(sql) {
     this.setState({ sql });
+    // Request server-side validation of the query text
+    if (this.canValidateQuery()) {
+      // NB. requestValidation is debounced
+      this.requestValidation();
+    }
   }
   // One layer of abstraction for easy spying in unit tests
   getSqlEditorHeight() {
@@ -186,6 +198,28 @@ class SqlEditor extends React.PureComponent {
       [dimension]: `calc(${elementSize}% - ${gutterSize + GUTTER_MARGIN}px)`,
     };
   }
+  requestValidation() {
+    if (this.props.database) {
+      const qe = this.props.queryEditor;
+      const query = {
+        dbId: qe.dbId,
+        sql: this.state.sql,
+        sqlEditorId: qe.id,
+        schema: qe.schema,
+        templateParams: qe.templateParams,
+      };
+      this.props.actions.validateQuery(query);
+    }
+  }
+  canValidateQuery() {
+    // Check whether or not we can validate the current query based on whether
+    // or not the backend has a validator configured for it.
+    const validatorMap = window.featureFlags.SQL_VALIDATORS_BY_ENGINE;
+    if (this.props.database && validatorMap != null) {
+      return validatorMap.hasOwnProperty(this.props.database.backend);
+    }
+    return false;
+  }
   runQuery() {
     if (this.props.database) {
       this.startQuery(this.props.database.allow_run_async);

superset/assets/src/SqlLab/reducers/getInitialState.js

@@ -30,6 +30,11 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
     autorun: false,
     dbId: defaultDbId,
     queryLimit: restBootstrapData.common.conf.DEFAULT_SQLLAB_LIMIT,
+    validationResult: {
+      id: null,
+      errors: [],
+      completed: false,
+    },
   };
 
   return {

superset/assets/src/SqlLab/reducers/sqlLab.js

@@ -141,6 +141,71 @@ export default function sqlLabReducer(state = {}, action) {
     [actions.REMOVE_TABLE]() {
       return removeFromArr(state, 'tables', action.table);
     },
+    [actions.START_QUERY_VALIDATION]() {
+      let newState = Object.assign({}, state);
+      const sqlEditor = { id: action.query.sqlEditorId };
+      newState = alterInArr(newState, 'queryEditors', sqlEditor, {
+        validationResult: {
+          id: action.query.id,
+          errors: [],
+          completed: false,
+        },
+      });
+      return newState;
+    },
+    [actions.QUERY_VALIDATION_RETURNED]() {
+      // If the server is very slow about answering us, we might get validation
+      // responses back out of order. This check confirms the response we're
+      // handling corresponds to the most recently dispatched request.
+      //
+      // We don't care about any but the most recent because validations are
+      // only valid for the SQL text they correspond to -- once the SQL has
+      // changed, the old validation doesn't tell us anything useful anymore.
+      const qe = getFromArr(state.queryEditors, action.query.sqlEditorId);
+      if (qe.validationResult.id !== action.query.id) {
+        return state;
+      }
+      // Otherwise, persist the results on the queryEditor state
+      let newState = Object.assign({}, state);
+      const sqlEditor = { id: action.query.sqlEditorId };
+      newState = alterInArr(newState, 'queryEditors', sqlEditor, {
+        validationResult: {
+          id: action.query.id,
+          errors: action.results,
+          completed: true,
+        },
+      });
+      return newState;
+    },
+    [actions.QUERY_VALIDATION_FAILED]() {
+      // If the server is very slow about answering us, we might get validation
+      // responses back out of order. This check confirms the response we're
+      // handling corresponds to the most recently dispatched request.
+      //
+      // We don't care about any but the most recent because validations are
+      // only valid for the SQL text they correspond to -- once the SQL has
+      // changed, the old validation doesn't tell us anything useful anymore.
+      const qe = getFromArr(state.queryEditors, action.query.sqlEditorId);
+      if (qe.validationResult.id !== action.query.id) {
+        return state;
+      }
+      // Otherwise, persist the results on the queryEditor state
+      let newState = Object.assign({}, state);
+      const sqlEditor = { id: action.query.sqlEditorId };
+      newState = alterInArr(newState, 'queryEditors', sqlEditor, {
+        validationResult: {
+          id: action.query.id,
+          errors: [{
+            line_number: 1,
+            start_column: 1,
+            end_column: 1,
+            message: `The server failed to validate your query.\n${action.message}`,
+          }],
+          completed: true,
+        },
+      });
+      return newState;
+    },
     [actions.START_QUERY]() {
       let newState = Object.assign({}, state);
       if (action.query.sqlEditorId) {

superset/assets/src/featureFlags.ts

@@ -23,6 +23,7 @@ export enum FeatureFlag {
   OMNIBAR = 'OMNIBAR',
   CLIENT_CACHE = 'CLIENT_CACHE',
   SCHEDULED_QUERIES = 'SCHEDULED_QUERIES',
+  SQL_VALIDATORS_BY_ENGINE = 'SQL_VALIDATORS_BY_ENGINE',
 }
 
 export type FeatureFlagMap = {

superset/views/core.py

@@ -2585,9 +2585,8 @@ def validate_sql_json(self):
         except Exception as e:
             logging.exception(e)
             msg = _(
-                'Failed to validate your SQL query text. Please check that '
-                f'you have configured the {validator.name} validator '
-                'correctly and that any services it depends on are up. '
+                f'{validator.name} was unable to check your query.\nPlease '
+                'make sure that any services it depends on are available\n'
                 f'Exception: {e}')
             return json_error_response(f'{msg}')