Skip to content

Commit

Permalink
[WIP] Live query validation, where supported
Browse files Browse the repository at this point in the history
This builds on apache#7422 to build check-as-you-type sql
query validation in Sql Lab. This closes apache#6707 too.

It adds a (debounced) call to the validate_sql_json
API endpoint with the querytext, and on Lyft infra is
able to return feedback to the user (end to end) in
$TBD seconds.

At present feedback is provided only through the
"annotations" mechanism build in to ACE, although
I'd be open to adding full text elsewhere on the
page if there's interest.
  • Loading branch information
Alex Berghage committed May 6, 2019
1 parent 1c4e4dc commit b802506
Showing 7 changed files with 187 additions and 3 deletions.
65 changes: 65 additions & 0 deletions superset/assets/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
@@ -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,26 @@ export function resetState() {
return { type: RESET_STATE };
}

export function startQueryValidation(query) {
if (query == null) {
const msg = "programmer error: tried to validate a null query object";
console.error(msg);
throw msg;
}
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 +211,47 @@ export function runQuery(query) {
};
}

export function validateQuery(query) {
return function (dispatch) {
if (query == null) {
const msg = "programmer error: tried to validate a null query object";
console.error(msg);
throw msg;
}

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({
14 changes: 14 additions & 0 deletions superset/assets/src/SqlLab/components/AceEditorWrapper.jsx
Original file line number Diff line number Diff line change
@@ -157,6 +157,19 @@ class AceEditorWrapper extends React.PureComponent {
}
});
}
getAceAnnotations() {
const validationResult = this.props.queryEditor.validationResult;
if (validationResult.completed && validationResult.errors.length > 0) {
let 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 +183,7 @@ class AceEditorWrapper extends React.PureComponent {
editorProps={{ $blockScrolling: true }}
enableLiveAutocompletion
value={this.state.sql}
annotations={this.getAceAnnotations()}
/>
);
}
34 changes: 34 additions & 0 deletions superset/assets/src/SqlLab/components/SqlEditor.jsx
Original file line number Diff line number Diff line change
@@ -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);
5 changes: 5 additions & 0 deletions superset/assets/src/SqlLab/reducers/getInitialState.js
Original file line number Diff line number Diff line change
@@ -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 {
66 changes: 66 additions & 0 deletions superset/assets/src/SqlLab/reducers/sqlLab.js
Original file line number Diff line number Diff line change
@@ -141,6 +141,72 @@ 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 errors = action.results
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) {
1 change: 1 addition & 0 deletions superset/assets/src/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -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 = {
5 changes: 2 additions & 3 deletions superset/views/core.py
Original file line number Diff line number Diff line change
@@ -2577,9 +2577,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}')

0 comments on commit b802506

Please sign in to comment.