Skip to content

Commit

Permalink
perf(sqllab): Rendering perf improvement using immutable state (#20877)
Browse files Browse the repository at this point in the history
* perf(sqllab): Rendering perf improvement using immutable state

- keep queryEditors immutable during active state
- add unsavedQueryEditor to store all active changes
- refactor each component to subscribe the related unsaved editor state only

* revert ISaveableDatasource type cast

* missing trigger prop

* a default of an empty object and optional operator
  • Loading branch information
justinpark committed Aug 23, 2022
1 parent 4ca4a5c commit f77b910
Show file tree
Hide file tree
Showing 32 changed files with 1,929 additions and 606 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ describe('SqlLab query tabs', () => {
const initialUntitledCount = Math.max(
0,
...tabs
.map((i, tabItem) =>
Number(tabItem.textContent?.match(/Untitled Query (\d+)/)?.[1]),
.map(
(i, tabItem) =>
Number(tabItem.textContent?.match(/Untitled Query (\d+)/)?.[1]) ||
0,
)
.toArray(),
);
Expand Down
9 changes: 9 additions & 0 deletions superset-frontend/src/SqlLab/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ const sqlLabPersistStateConfig = {
...state[path],
queries: emptyQueryResults(state[path].queries),
queryEditors: clearQueryEditors(state[path].queryEditors),
unsavedQueryEditor: clearQueryEditors([
state[path].unsavedQueryEditor,
])[0],
};
}
});
Expand All @@ -91,6 +94,12 @@ const sqlLabPersistStateConfig = {
const result = {
...initialState,
...persistedState,
sqlLab: {
...(persistedState?.sqlLab || {}),
// Overwrite initialState over persistedState for sqlLab
// since a logic in getInitialState overrides the value from persistedState
...initialState.sqlLab,
},
};
// Filter out any user data that may have been persisted in an older version.
// Get user from bootstrap data instead, every time
Expand Down
102 changes: 86 additions & 16 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,17 @@ const fieldConverter = mapping => obj =>
const convertQueryToServer = fieldConverter(queryServerMapping);
const convertQueryToClient = fieldConverter(queryClientMapping);

export function getUpToDateQuery(rootState, queryEditor, key) {
const {
sqlLab: { unsavedQueryEditor },
} = rootState;
const id = key ?? queryEditor.id;
return {
...queryEditor,
...(id === unsavedQueryEditor.id && unsavedQueryEditor),
};
}

export function resetState() {
return { type: RESET_STATE };
}
Expand Down Expand Up @@ -167,24 +178,26 @@ export function scheduleQuery(query) {
);
}

export function estimateQueryCost(query) {
const { dbId, schema, sql, templateParams } = query;
const endpoint =
schema === null
? `/superset/estimate_query_cost/${dbId}/`
: `/superset/estimate_query_cost/${dbId}/${schema}/`;
return dispatch =>
Promise.all([
dispatch({ type: COST_ESTIMATE_STARTED, query }),
export function estimateQueryCost(queryEditor) {
return (dispatch, getState) => {
const { dbId, schema, sql, selectedText, templateParams } =
getUpToDateQuery(getState(), queryEditor);
const requestSql = selectedText || sql;
const endpoint =
schema === null
? `/superset/estimate_query_cost/${dbId}/`
: `/superset/estimate_query_cost/${dbId}/${schema}/`;
return Promise.all([
dispatch({ type: COST_ESTIMATE_STARTED, query: queryEditor }),
SupersetClient.post({
endpoint,
postPayload: {
sql,
sql: requestSql,
templateParams: JSON.parse(templateParams || '{}'),
},
})
.then(({ json }) =>
dispatch({ type: COST_ESTIMATE_RETURNED, query, json }),
dispatch({ type: COST_ESTIMATE_RETURNED, query: queryEditor, json }),
)
.catch(response =>
getClientErrorObject(response).then(error => {
Expand All @@ -194,12 +207,13 @@ export function estimateQueryCost(query) {
t('Failed at retrieving results');
return dispatch({
type: COST_ESTIMATE_FAILED,
query,
query: queryEditor,
error: message,
});
}),
),
]);
};
}

export function startQuery(query) {
Expand Down Expand Up @@ -357,15 +371,58 @@ export function runQuery(query) {
};
}

export function runQueryFromSqlEditor(
database,
queryEditor,
defaultQueryLimit,
tempTable,
ctas,
ctasMethod,
) {
return function (dispatch, getState) {
const qe = getUpToDateQuery(getState(), queryEditor, queryEditor.id);
const query = {
dbId: qe.dbId,
sql: qe.selectedText || qe.sql,
sqlEditorId: qe.id,
tab: qe.name,
schema: qe.schema,
tempTable,
templateParams: qe.templateParams,
queryLimit: qe.queryLimit || defaultQueryLimit,
runAsync: database ? database.allow_run_async : false,
ctas,
ctas_method: ctasMethod,
updateTabState: !qe.selectedText,
};
dispatch(runQuery(query));
};
}

export function reRunQuery(query) {
// run Query with a new id
return function (dispatch) {
dispatch(runQuery({ ...query, id: shortid.generate() }));
};
}

export function validateQuery(query) {
return function (dispatch) {
export function validateQuery(queryEditor, sql) {
return function (dispatch, getState) {
const {
sqlLab: { unsavedQueryEditor },
} = getState();
const qe = {
...queryEditor,
...(queryEditor.id === unsavedQueryEditor.id && unsavedQueryEditor),
};

const query = {
dbId: qe.dbId,
sql,
sqlEditorId: qe.id,
schema: qe.schema,
templateParams: qe.templateParams,
};
dispatch(startQueryValidation(query));

const postPayload = {
Expand Down Expand Up @@ -620,6 +677,7 @@ export function switchQueryEditor(queryEditor, displayLimit) {
return function (dispatch) {
if (
isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) &&
queryEditor &&
!queryEditor.loaded
) {
SupersetClient.get({
Expand Down Expand Up @@ -723,6 +781,17 @@ export function removeQueryEditor(queryEditor) {
};
}

export function removeAllOtherQueryEditors(queryEditor) {
return function (dispatch, getState) {
const { sqlLab } = getState();
sqlLab.queryEditors?.forEach(otherQueryEditor => {
if (otherQueryEditor.id !== queryEditor.id) {
dispatch(removeQueryEditor(otherQueryEditor));
}
});
};
}

export function removeQuery(query) {
return function (dispatch) {
const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
Expand Down Expand Up @@ -921,8 +990,9 @@ export function queryEditorSetSql(queryEditor, sql) {
return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql };
}

export function queryEditorSetAndSaveSql(queryEditor, sql) {
return function (dispatch) {
export function queryEditorSetAndSaveSql(targetQueryEditor, sql) {
return function (dispatch, getState) {
const queryEditor = getUpToDateQuery(getState(), targetQueryEditor);
// saved query and set tab state use this action
dispatch(queryEditorSetSql(queryEditor, sql));
if (isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) {
Expand Down

0 comments on commit f77b910

Please sign in to comment.