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.

Sign up for GitHub

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

[explore view] fix long query issue from Run in SQL LAB Button #9345

Merged
merged 4 commits into from
Mar 25, 2020
Merged

[explore view] fix long query issue from Run in SQL LAB Button #9345

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d20d31
[explore view] fix long query issue from Run in SQL LAB Button
Mar 21, 2020
a64aaf9
SQL Lab page needs to take the post form data, too
ktmud Mar 22, 2020
bd3b854
fix variable names
Mar 25, 2020
a2bbc45
updated payload dict, rename hidden form
Mar 25, 2020
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
3 changes: 2 additions & 1 deletion superset-frontend/src/SqlLab/App.jsx
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
21 changes: 14 additions & 7 deletions superset-frontend/src/explore/exploreUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,17 +190,15 @@ 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';
exploreForm.target = target;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rename exploreForm to something more generic. E.g., hiddenForm.

const token = document.createElement('input');
token.type = 'hidden';
token.name = 'csrf_token';
Expand All @@ -216,3 +214,12 @@ export function exportChart(formData, endpointType) {
exploreForm.submit();
document.body.removeChild(exploreForm);
}

export function exportChart(formData, endpointType) {
const { url, payload } = getExploreUrlAndPayload({
formData,
endpointType,
allowDomainSharding: false,
});
postForm(url, payload);
}
21 changes: 16 additions & 5 deletions superset/views/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2703,7 +2703,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 @@ -2743,19 +2743,30 @@ 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"])
Copy link
Author

Choose a reason for hiding this comment

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

this is to make /sqllab page accept POST

Copy link
Member

Choose a reason for hiding this comment

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

The request is accepted but the post data is not used, hence the SQL query is actually not pre-populated in the SQL editor as it is now. Previously the search query is acquired at the client side: cefedf6#diff-2782bdc5cd0627eef4679c85a44d4ac1L102

Copy link
Author

Choose a reason for hiding this comment

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

i merged your code snippet. now it works. thanks!

def sqllab(self):
"""SQL Editor"""
payload = self._get_sqllab_payload(g.user.get_id())
payload = {
"defaultDbId": config["SQLLAB_DEFAULT_DBID"],
"common": common_bootstrap_payload(),
}

tabs_data = self._get_sqllab_tabs(g.user.get_id())
payload.update(tabs_data)
Copy link
Member

Choose a reason for hiding this comment

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

Can rewrite with dict unpacking:

        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)
Copy link
Author

@graceguo-supercat graceguo-supercat Mar 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

i prefer call it requested_query not form_data. form_data is used in dashboard and explore view, just feel this name is over loaded.

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,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