Skip to content

Commit

Permalink
Revert "Speed up JSON; partial loading of JSON query results in React (
Browse files Browse the repository at this point in the history
…getredash#2)"

This reverts commit 21faf54.
  • Loading branch information
shashwatSmartcoin authored Mar 6, 2024
1 parent 21faf54 commit 8a40c9d
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 130 deletions.
3 changes: 1 addition & 2 deletions client/app/lib/useQueryResultData.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,5 @@ function getQueryResultData(queryResult, queryResultStatus = null) {
export default function useQueryResultData(queryResult) {
// make sure it re-executes when queryResult status changes
const queryResultStatus = invoke(queryResult, "getStatus");
const queryResultRowCount = get(queryResult, "query_result.data.rows.length");
return useMemo(() => getQueryResultData(queryResult, queryResultStatus), [queryResult, queryResultStatus, queryResultRowCount]);
return useMemo(() => getQueryResultData(queryResult, queryResultStatus), [queryResult, queryResultStatus]);
}
30 changes: 13 additions & 17 deletions client/app/pages/alert/Alert.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,6 @@ class Alert extends React.Component {
});
};

handleResult = queryResult => {
if (this._isMounted) {
this.setState({ queryResult });
let { column } = this.state.alert.options;
const columns = queryResult.getColumnNames();

// default to first column name if none chosen, or irrelevant in current query
if (!column || !includes(columns, column)) {
column = head(queryResult.getColumnNames());
}
this.setAlertOptions({ column });
}
}

onQuerySelected = query => {
this.setState(({ alert }) => ({
alert: Object.assign(alert, { query }),
Expand All @@ -144,9 +130,19 @@ class Alert extends React.Component {

if (query) {
// get cached result for column names and values
const promises = new QueryService(query).getQueryResultPromises()
promises[0].then(queryResult => this.handleResult(queryResult));
promises[1].then(queryResult => this.handleResult(queryResult));
new QueryService(query).getQueryResultPromise().then(queryResult => {
if (this._isMounted) {
this.setState({ queryResult });
let { column } = this.state.alert.options;
const columns = queryResult.getColumnNames();

// default to first column name if none chosen, or irrelevant in current query
if (!column || !includes(columns, column)) {
column = head(queryResult.getColumnNames());
}
this.setAlertOptions({ column });
}
});
}
};

Expand Down
2 changes: 0 additions & 2 deletions client/app/pages/queries/QueryView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ function QueryView(props) {
const {
queryResult,
loadedInitialResults,
loadedFullResults,
isExecuting,
executionStatus,
executeQuery,
Expand Down Expand Up @@ -187,7 +186,6 @@ function QueryView(props) {
canRefresh={policy.canRun(query)}
/>
)}
{loadedInitialResults && !loadedFullResults && <div style={{color: "orange", textAlign: "center"}}>Still loading more results...</div>}
<div className="query-results-footer">
{queryResult && !queryResult.getError() && (
<QueryExecutionMetadata
Expand Down
84 changes: 38 additions & 46 deletions client/app/pages/queries/hooks/useQueryExecute.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export default function useQueryExecute(query) {
queryResult: null,
isExecuting: false,
loadedInitialResults: false,
loadedFullResults: false,
executionStatus: null,
isCancelling: false,
cancelCallback: null,
Expand Down Expand Up @@ -68,53 +67,46 @@ export default function useQueryExecute(query) {
}
};

const successResult = (queryResult, instance) => {
if (queryResultInExecution.current === newQueryResult) {
// TODO: this should probably belong in the QueryEditor page.
if (queryResult && queryResult.query_result.query === query.query) {
query.latest_query_data_id = queryResult.getId();
query.queryResult = queryResult;
newQueryResult
.toPromise(onStatusChange)
.then(queryResult => {
if (queryResultInExecution.current === newQueryResult) {
// TODO: this should probably belong in the QueryEditor page.
if (queryResult && queryResult.query_result.query === query.query) {
query.latest_query_data_id = queryResult.getId();
query.queryResult = queryResult;
}

if (executionState.loadedInitialResults) {
notifications.showNotification("Redash", `${query.name} updated.`);
}

setExecutionState({
queryResult,
loadedInitialResults: true,
error: null,
isExecuting: false,
isCancelling: false,
executionStatus: null,
});
}

if (executionState.loadedInitialResults) {
notifications.showNotification("Redash", `${query.name} updated.`);
})
.catch(queryResult => {
if (queryResultInExecution.current === newQueryResult) {
if (executionState.loadedInitialResults) {
notifications.showNotification("Redash", `${query.name} failed to run: ${queryResult.getError()}`);
}

setExecutionState({
queryResult,
loadedInitialResults: true,
error: queryResult.getError(),
isExecuting: false,
isCancelling: false,
executionStatus: ExecutionStatus.FAILED,
});
}

setExecutionState({
queryResult,
loadedInitialResults: true,
loadedFullResults: instance > 0,
error: null,
isExecuting: false,
isCancelling: false,
executionStatus: null,
});
}
}

const errorResult = queryResult => {
if (queryResultInExecution.current === newQueryResult) {
if (executionState.loadedInitialResults) {
notifications.showNotification("Redash", `${query.name} failed to run: ${queryResult.getError()}`);
}

setExecutionState({
queryResult,
loadedInitialResults: true,
loadedFullResults: true,
error: queryResult.getError(),
isExecuting: false,
isCancelling: false,
executionStatus: ExecutionStatus.FAILED,
});
}
}

const promises = newQueryResult.toPromise(onStatusChange);
promises[0].then(queryResult => successResult(queryResult, 0)).catch(queryResult => errorResult(queryResult));
if (promises[1]) {
promises[1].then(queryResult => successResult(queryResult, 1)).catch(queryResult => errorResult(queryResult));
}
});
});

const queryRef = useRef(query);
Expand Down
65 changes: 22 additions & 43 deletions client/app/services/query-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,12 @@ import { isString, uniqBy, each, isNumber, includes, extend, forOwn, get } from
const logger = debug("redash:services:QueryResult");
const filterTypes = ["filter", "multi-filter", "multiFilter"];

function defer(instance = 0) {
function defer() {
const result = { onStatusChange: status => {} };
if (instance > 0) {
result.promise2 = new Promise((resolve, reject) => {
result.resolve2 = resolve;
result.reject2 = reject;
});
} else {
result.promise = new Promise((resolve, reject) => {
result.resolve = resolve;
result.reject = reject;
});
}
result.promise = new Promise((resolve, reject) => {
result.resolve = resolve;
result.reject = reject;
});
return result;
}

Expand Down Expand Up @@ -71,7 +64,7 @@ const statuses = {
4: ExecutionStatus.FAILED,
};

function handleErrorResponse(queryResult, error, instance = 0) {
function handleErrorResponse(queryResult, error) {
const status = get(error, "response.status");
switch (status) {
case 403:
Expand Down Expand Up @@ -100,7 +93,7 @@ function handleErrorResponse(queryResult, error, instance = 0) {
error: get(error, "response.data.message", "Unknown error occurred. Please try again later."),
status: 4,
},
}, instance);
});
}

function sleep(ms) {
Expand All @@ -122,8 +115,7 @@ export function fetchDataFromJob(jobId, interval = 1000) {

class QueryResult {
constructor(props) {
this.deferred = defer(0);
this.deferred2 = defer(1);
this.deferred = defer();
this.job = {};
this.query_result = {};
this.status = "waiting";
Expand All @@ -138,12 +130,12 @@ class QueryResult {
}
}

update(props, instance = 0) {
update(props) {
extend(this, props);

if ("query_result" in props) {
this.status = ExecutionStatus.DONE;
(instance > 0 ? this.deferred : this.deferred2).onStatusChange(ExecutionStatus.DONE);
this.deferred.onStatusChange(ExecutionStatus.DONE);

const columnTypes = {};

Expand Down Expand Up @@ -186,15 +178,15 @@ class QueryResult {
}
});

instance > 0 ? this.deferred2.resolve2(this) : this.deferred.resolve(this);
this.deferred.resolve(this);
} else if (this.job.status === 3 || this.job.status === 2) {
(instance > 0 ? this.deferred2 : this.deferred).onStatusChange(ExecutionStatus.PROCESSING);
this.deferred.onStatusChange(ExecutionStatus.PROCESSING);
this.status = "processing";
} else if (this.job.status === 4) {
this.status = statuses[this.job.status];
(instance > 0 ? this.deferred2.reject2 : this.deferred.reject)(new QueryResultError(this.job.error));
this.deferred.reject(new QueryResultError(this.job.error));
} else {
(instance > 0 ? this.deferred2 : this.deferred).onStatusChange(undefined);
this.deferred.onStatusChange(undefined);
this.status = undefined;
}
}
Expand Down Expand Up @@ -335,39 +327,26 @@ class QueryResult {
if (statusCallback) {
this.deferred.onStatusChange = statusCallback;
}
return [this.deferred.promise, this.deferred2.promise2];
return this.deferred.promise;
}

static getById(queryId, id, partial = false) {
static getById(queryId, id) {
const queryResult = new QueryResult();

queryResult.isLoadingResult = true;
queryResult.deferred.onStatusChange(ExecutionStatus.LOADING_RESULT);

axios
.get(`api/queries/${queryId}/results/${id}.json?partial=${partial}`)
.get(`api/queries/${queryId}/results/${id}.json`)
.then(response => {
// Success handler
queryResult.isLoadingResult = false;
queryResult.update(response, 0);

axios
.get(`api/queries/${queryId}/results/${id}.json?partial=false`)
.then(response => {
// Success handler
queryResult.isLoadingResult = false;
queryResult.update(response, 1);
})
.catch(error => {
// Error handler
queryResult.isLoadingResult = false;
handleErrorResponse(queryResult, error, 1);
});
queryResult.update(response);
})
.catch(error => {
// Error handler
queryResult.isLoadingResult = false;
handleErrorResponse(queryResult, error, 0);
handleErrorResponse(queryResult, error);
});

return queryResult;
Expand All @@ -384,11 +363,11 @@ class QueryResult {
});
}

loadResult(tryCount, first = true) {
loadResult(tryCount) {
this.isLoadingResult = true;
this.deferred.onStatusChange(ExecutionStatus.LOADING_RESULT);

QueryResultResource.get({ id: this.job.query_result_id, partial: first })
QueryResultResource.get({ id: this.job.query_result_id })
.then(response => {
this.update(response);
this.isLoadingResult = false;
Expand All @@ -409,7 +388,7 @@ class QueryResult {
this.isLoadingResult = false;
} else {
setTimeout(() => {
this.loadResult(tryCount + 1, false);
this.loadResult(tryCount + 1);
}, 1000 * Math.pow(2, tryCount));
}
});
Expand Down
6 changes: 3 additions & 3 deletions client/app/services/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export class Query {
}
} else if (this.latest_query_data_id && maxAge !== 0) {
if (!this.queryResult) {
this.queryResult = QueryResult.getById(this.id, this.latest_query_data_id, true);
this.queryResult = QueryResult.getById(this.id, this.latest_query_data_id);
}
} else {
this.queryResult = execute();
Expand Down Expand Up @@ -176,7 +176,7 @@ export class Query {
return url;
}

getQueryResultPromises() {
getQueryResultPromise() {
return this.getQueryResult().toPromise();
}

Expand Down Expand Up @@ -355,7 +355,7 @@ export class QueryResultError {
}

toPromise() {
return [Promise.reject(this)];
return Promise.reject(this);
}

// eslint-disable-next-line class-methods-use-this
Expand Down
4 changes: 2 additions & 2 deletions client/app/services/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class Widget {
this.queryResult = queryResult;

queryResult
.toPromise()[0]
.toPromise()
.then(result => {
if (this.queryResult === queryResult) {
this.loading = false;
Expand All @@ -180,7 +180,7 @@ class Widget {
});
}

return this.queryResult.toPromise()[0];
return this.queryResult.toPromise();
}

save(key, value) {
Expand Down
Loading

0 comments on commit 8a40c9d

Please sign in to comment.