codeant-ai-for-open-source[bot] commented on code in PR #40907:
URL: https://github.com/apache/superset/pull/40907#discussion_r4114448089
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -62,6 +64,13 @@ export const useResultsPane = ({
const chartRowLimit = Number(queryFormData?.row_limit) || 10000;
const [rowLimit, setRowLimit] = useState(1000);
+ const [orderby, setOrderby] = useState<[string, boolean][]>([]);
+ // Server-side sort is only valid when the displayed columns map directly to
+ // the SQL result. When the query has post-processing (e.g.
pivot/cum/rolling),
+ // `orderby` + `row_limit` are applied to the raw SQL *before*
post-processing,
+ // which changes the rows that feed those operations and corrupts the result.
+ // In that case we fall back to client-side sorting of what the chart
produced.
+ const [hasPostProcessing, setHasPostProcessing] = useState(false);
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `1dd4b0b`.
`hasPostProcessing` now initializes to `true`, disabling server-side sorting
until asynchronous detection confirms that post-processing is absent.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -86,6 +101,10 @@ export const useResultsPane = ({
[cappedFormData],
);
+ const handleServerSort = useCallback((nextOrderby: [string, boolean][]) => {
+ setOrderby(nextOrderby);
+ }, []);
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `1dd4b0b`.
The request effect now increments `latestRequestId` and associates each
request with its ID so stale responses can be ignored when a newer sort or
row-limit request supersedes them.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:
##########
@@ -308,7 +328,10 @@ export default function DrillDetailPane({
useEffect(() => {
if (!responseError && !isLoading && !resultsPages.has(pageIndex)) {
setIsLoading(true);
- const jsonPayload = getDrillPayload(formData, filters) ?? {};
+ const jsonPayload = {
+ ...getDrillPayload(formData, filters),
+ ...(orderby.length > 0 && { orderby }),
+ };
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `1dd4b0b`.
Each drill-detail request captures the current `sortGeneration`, and the
response returns without updating `resultsPages` when that generation is no
longer current.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -74,6 +76,13 @@ export const useResultsPane = ({
const chartRowLimit = Number(queryFormData?.row_limit) || 10000;
const [rowLimit, setRowLimit] = useState(1000);
+ const [orderby, setOrderby] = useState<[string, boolean][]>([]);
+ // Server-side sort is only valid when the displayed columns map directly to
+ // the SQL result. When the query has post-processing (e.g.
pivot/cum/rolling),
+ // `orderby` + `row_limit` are applied to the raw SQL *before*
post-processing,
+ // which changes the rows that feed those operations and corrupts the result.
+ // In that case we fall back to client-side sorting of what the chart
produced.
+ const [hasPostProcessing, setHasPostProcessing] = useState(false);
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `1dd4b0b`.
Post-processing detection now starts with `hasPostProcessing` set to `true`,
keeping server-side sorting disabled while detection is pending.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -109,6 +124,10 @@ export const useResultsPane = ({
[cappedFormData],
);
+ const handleServerSort = useCallback((nextOrderby: [string, boolean][]) => {
+ setOrderby(nextOrderby);
+ }, []);
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `1dd4b0b`.
The results request flow now tracks `latestRequestId` to distinguish
superseded requests and prevent older sort responses from replacing current
results.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
