Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Live query validation in the SQL Lab UI #7461

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions superset/assets/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
bearcage marked this conversation as resolved.
Show resolved Hide resolved
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({
Expand Down Expand Up @@ -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);
bearcage marked this conversation as resolved.
Show resolved Hide resolved
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({
Expand Down
14 changes: 14 additions & 0 deletions superset/assets/src/SqlLab/components/AceEditorWrapper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -170,6 +183,7 @@ class AceEditorWrapper extends React.PureComponent {
editorProps={{ $blockScrolling: true }}
enableLiveAutocompletion
value={this.state.sql}
annotations={this.getAceAnnotations()}
/>
);
}
Expand Down
34 changes: 34 additions & 0 deletions superset/assets/src/SqlLab/components/SqlEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is NB?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never realized that was weird, but I guess it is a bit now that I think about it!

NB means "nota bene", latin for "note well" — I usually use it in comments where "NOTE:" would also work

this.requestValidation();
}
}
// One layer of abstraction for easy spying in unit tests
getSqlEditorHeight() {
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions superset/assets/src/SqlLab/reducers/getInitialState.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
66 changes: 66 additions & 0 deletions superset/assets/src/SqlLab/reducers/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions superset/assets/src/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
5 changes: 2 additions & 3 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}')

Expand Down