Skip to content

Commit

Permalink
[explore view] fix long query issue from Run in SQL LAB Button (#9345)
Browse files Browse the repository at this point in the history
* [explore view] fix long query issue from Run in SQL LAB Button

* SQL Lab page needs to take the post form data, too

* fix variable names

* updated payload dict, rename hidden form

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>
  • Loading branch information
Grace Guo and ktmud authored Mar 25, 2020
1 parent 43f0221 commit 6b0f62a
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 44 deletions.
3 changes: 2 additions & 1 deletion superset-frontend/src/SqlLab/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ setupApp();

const appContainer = document.getElementById('app');
const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));

initFeatureFlags(bootstrapData.common.feature_flags);

const initialState = getInitialState(bootstrapData);
const sqlLabPersistStateConfig = {
paths: ['sqlLab'],
Expand All @@ -59,7 +61,6 @@ const sqlLabPersistStateConfig = {
// it caused configurations passed from server-side got override.
// see PR 6257 for details
delete state[path].common; // eslint-disable-line no-param-reassign

if (path === 'sqlLab') {
subset[path] = {
...state[path],
Expand Down
12 changes: 10 additions & 2 deletions superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const propTypes = {
databases: PropTypes.object.isRequired,
queries: PropTypes.object.isRequired,
queryEditors: PropTypes.array,
requestedQuery: PropTypes.object,
tabHistory: PropTypes.array.isRequired,
tables: PropTypes.array.isRequired,
offline: PropTypes.bool,
Expand All @@ -48,6 +49,7 @@ const propTypes = {
const defaultProps = {
queryEditors: [],
offline: false,
requestedQuery: null,
saveQueryWarning: null,
scheduleQueryWarning: null,
};
Expand Down Expand Up @@ -99,7 +101,12 @@ class TabbedSqlEditors extends React.PureComponent {
});
}

const query = URI(window.location).search(true);
// merge post form data with GET search params
const query = {
...this.props.requestedQuery,
...URI(window.location).search(true),
};

// Popping a new tab based on the querystring
if (query.id || query.sql || query.savedQueryId || query.datasourceKey) {
if (query.id) {
Expand Down Expand Up @@ -374,7 +381,7 @@ class TabbedSqlEditors extends React.PureComponent {
TabbedSqlEditors.propTypes = propTypes;
TabbedSqlEditors.defaultProps = defaultProps;

function mapStateToProps({ sqlLab, common }) {
function mapStateToProps({ sqlLab, common, requestedQuery }) {
return {
databases: sqlLab.databases,
queryEditors: sqlLab.queryEditors,
Expand All @@ -388,6 +395,7 @@ function mapStateToProps({ sqlLab, common }) {
maxRow: common.conf.SQL_MAX_ROW,
saveQueryWarning: common.conf.SQLLAB_SAVE_WARNING_MESSAGE,
scheduleQueryWarning: common.conf.SQLLAB_SCHEDULE_WARNING_MESSAGE,
requestedQuery,
};
}
function mapDispatchToProps(dispatch) {
Expand Down
33 changes: 21 additions & 12 deletions superset-frontend/src/SqlLab/reducers/getInitialState.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@
import { t } from '@superset-ui/translation';
import getToastsFromPyFlashMessages from '../../messageToasts/utils/getToastsFromPyFlashMessages';

export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
/*
export default function getInitialState({
defaultDbId,
common,
active_tab: activeTab,
tab_state_ids: tabStateIds = [],
databases,
queries: queries_,
requested_query: requestedQuery,
}) {
/**
* Before YYYY-MM-DD, the state for SQL Lab was stored exclusively in the
* browser's localStorage. The feature flag `SQLLAB_BACKEND_PERSISTENCE`
* moves the state to the backend instead, migrating it from local storage.
Expand All @@ -39,7 +47,7 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
autorun: false,
templateParams: null,
dbId: defaultDbId,
queryLimit: restBootstrapData.common.conf.DEFAULT_SQLLAB_LIMIT,
queryLimit: common.conf.DEFAULT_SQLLAB_LIMIT,
validationResult: {
id: null,
errors: [],
Expand All @@ -52,11 +60,11 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
},
};

/* Load state from the backend. This will be empty if the feature flag
/**
* Load state from the backend. This will be empty if the feature flag
* `SQLLAB_BACKEND_PERSISTENCE` is off.
*/
const activeTab = restBootstrapData.active_tab;
restBootstrapData.tab_state_ids.forEach(({ id, label }) => {
tabStateIds.forEach(({ id, label }) => {
let queryEditor;
if (activeTab && activeTab.id === id) {
queryEditor = {
Expand Down Expand Up @@ -92,7 +100,6 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
});

const tabHistory = activeTab ? [activeTab.id.toString()] : [];

const tables = [];
if (activeTab) {
activeTab.table_schemas
Expand Down Expand Up @@ -126,9 +133,10 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
});
}

const { databases, queries } = restBootstrapData;
const queries = { ...queries_ };

/* If the `SQLLAB_BACKEND_PERSISTENCE` feature flag is off, or if the user
/**
* If the `SQLLAB_BACKEND_PERSISTENCE` feature flag is off, or if the user
* hasn't used SQL Lab after it has been turned on, the state will be stored
* in the browser's local storage.
*/
Expand Down Expand Up @@ -173,13 +181,14 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
tables,
queriesLastUpdate: Date.now(),
},
requestedQuery,
messageToasts: getToastsFromPyFlashMessages(
(restBootstrapData.common || {}).flash_messages || [],
(common || {}).flash_messages || [],
),
localStorageUsageInKilobytes: 0,
common: {
flash_messages: restBootstrapData.common.flash_messages,
conf: restBootstrapData.common.conf,
flash_messages: common.flash_messages,
conf: common.conf,
},
};
}
15 changes: 7 additions & 8 deletions superset-frontend/src/chart/chartAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import {
getExploreUrlAndPayload,
getAnnotationJsonUrl,
postForm,
} from '../explore/exploreUtils';
import {
requiresQuery,
Expand Down Expand Up @@ -358,14 +359,12 @@ export function redirectSQLLab(formData) {
postPayload: { form_data: formData },
})
.then(({ json }) => {
const redirectUrl = new URL(window.location);
redirectUrl.pathname = '/superset/sqllab';
for (const key of redirectUrl.searchParams.keys()) {
redirectUrl.searchParams.delete(key);
}
redirectUrl.searchParams.set('datasourceKey', formData.datasource);
redirectUrl.searchParams.set('sql', json.query);
window.open(redirectUrl.href, '_blank');
const redirectUrl = '/superset/sqllab';
const payload = {
datasourceKey: formData.datasource,
sql: json.query,
};
postForm(redirectUrl, payload);
})
.catch(() =>
dispatch(addDangerToast(t('An error occurred while loading the SQL'))),
Expand Down
37 changes: 22 additions & 15 deletions superset-frontend/src/explore/exploreUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,29 +190,36 @@ export function getExploreUrlAndPayload({
};
}

export function exportChart(formData, endpointType) {
const { url, payload } = getExploreUrlAndPayload({
formData,
endpointType,
allowDomainSharding: false,
});
export function postForm(url, payload, target = '_blank') {
if (!url) {
return;
}

const exploreForm = document.createElement('form');
exploreForm.action = url;
exploreForm.method = 'POST';
exploreForm.target = '_blank';
const hiddenForm = document.createElement('form');
hiddenForm.action = url;
hiddenForm.method = 'POST';
hiddenForm.target = target;
const token = document.createElement('input');
token.type = 'hidden';
token.name = 'csrf_token';
token.value = (document.getElementById('csrf_token') || {}).value;
exploreForm.appendChild(token);
hiddenForm.appendChild(token);
const data = document.createElement('input');
data.type = 'hidden';
data.name = 'form_data';
data.value = safeStringify(payload);
exploreForm.appendChild(data);
hiddenForm.appendChild(data);

document.body.appendChild(exploreForm);
exploreForm.submit();
document.body.removeChild(exploreForm);
document.body.appendChild(hiddenForm);
hiddenForm.submit();
document.body.removeChild(hiddenForm);
}

export function exportChart(formData, endpointType) {
const { url, payload } = getExploreUrlAndPayload({
formData,
endpointType,
allowDomainSharding: false,
});
postForm(url, payload);
}
19 changes: 14 additions & 5 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2713,7 +2713,7 @@ def profile(self, username):
)

@staticmethod
def _get_sqllab_payload(user_id: int) -> Dict[str, Any]:
def _get_sqllab_tabs(user_id: int) -> Dict[str, Any]:
# send list of tab state ids
tabs_state = (
db.session.query(TabState.id, TabState.label)
Expand Down Expand Up @@ -2753,19 +2753,28 @@ def _get_sqllab_payload(user_id: int) -> Dict[str, Any]:
}

return {
"defaultDbId": config["SQLLAB_DEFAULT_DBID"],
"common": common_bootstrap_payload(),
"tab_state_ids": tabs_state,
"active_tab": active_tab.to_dict() if active_tab else None,
"databases": databases,
"queries": queries,
}

@has_access
@expose("/sqllab")
@expose("/sqllab", methods=["GET", "POST"])
def sqllab(self):
"""SQL Editor"""
payload = self._get_sqllab_payload(g.user.get_id())
payload = {
"defaultDbId": config["SQLLAB_DEFAULT_DBID"],
"common": common_bootstrap_payload(),
**self._get_sqllab_tabs(g.user.get_id()),
}

form_data = request.form.get("form_data")
if form_data:
try:
payload["requested_query"] = json.loads(form_data)
except json.JSONDecodeError:
pass
bootstrap_data = json.dumps(
payload, default=utils.pessimistic_json_iso_dttm_ser
)
Expand Down
2 changes: 1 addition & 1 deletion tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ def test_sqllab_backend_persistence_payload(self):

# we should have only 1 query returned, since the second one is not
# associated with any tabs
payload = views.Superset._get_sqllab_payload(user_id=user_id)
payload = views.Superset._get_sqllab_tabs(user_id=user_id)
self.assertEqual(len(payload["queries"]), 1)


Expand Down

0 comments on commit 6b0f62a

Please sign in to comment.