Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Commit

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

* 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

(cherry picked from commit ddb5fcdc0cb597c2fd947d39a05055c2949c29a7)
  • Loading branch information
justinpark authored and john-bodley committed Aug 24, 2022
1 parent cc6f9ac commit 0cf7070
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 0cf7070

Please sign in to comment.