From 4489ebfcf3942551dcf0639350b67a7fcd67a112 Mon Sep 17 00:00:00 2001 From: dienaso <89768641@qq.com> Date: Fri, 9 Oct 2026 18:36:36 +0800 Subject: [PATCH] fix(grid): stop load-all re-requesting exhausted result sets --- apps/desktop/src/components/grid/DataGrid.vue | 29 +++++++++++- .../dataGrid/dataGridInfiniteScroll.spec.ts | 47 ++++++++++++++++++- .../src/lib/dataGrid/dataGridPagination.ts | 36 ++++++++++++++ 3 files changed, 109 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/components/grid/DataGrid.vue b/apps/desktop/src/components/grid/DataGrid.vue index 62905efeb8..5f5ae35a58 100644 --- a/apps/desktop/src/components/grid/DataGrid.vue +++ b/apps/desktop/src/components/grid/DataGrid.vue @@ -225,6 +225,7 @@ import { dataGridHeaderContentWidth, scrollbarGutterWidth } from "@/lib/dataGrid import { canFetchNextDataGridSegment, canGoNextDataGridPage, + canStartDataGridLoadAll, dataGridLoadAllInitialTarget, dataGridLoadAllNextSegment, dataGridLoadAllSegment, @@ -3878,7 +3879,20 @@ function nextLoadAllSegment() { return segment && !canFetchNextInfiniteScrollSegment.value ? { offset: segment.offset, limit: Math.min(segment.limit, pageSize.value) } : segment; } const effectiveTotal = paginationTotalRowCount.value ?? displayedTotalRowCount.value; - const canFetchMore = canFetchNextInfiniteScrollSegment.value && (!totalRowCountIsExact.value || effectiveTotal === undefined || props.result.rows.length < effectiveTotal); + // The "no more rows" marker must gate a re-request even when the total is + // unknown: re-fetching an exhausted result duplicated rows (#11321). The + // marker also gets set by the result-row cap; a capped result is not + // exhausted, and load-all is the only way past the cap (#10752). + const canFetchMore = canStartDataGridLoadAll({ + hasMore: props.result.has_more, + loadedRowCount: props.result.rows.length, + pageSize: pageSize.value, + totalRowCount: hasKnownPaginationTotalRowCount.value ? paginationTotalRowCount.value : undefined, + exactTotal: totalRowCountIsExact.value ? effectiveTotal : undefined, + allRowsLoaded: infiniteScrollAllLoaded, + localResultComplete: allRowsLoaded.value, + resultRowCap: infiniteScrollMaxRows.value, + }); const targetMaxRows = dataGridLoadAllInitialTarget(props.result.rows.length, infiniteScrollMaxRows.value, totalRowCountIsExact.value ? effectiveTotal : undefined); return dataGridLoadAllSegment(props.result.rows.length, targetMaxRows, canFetchMore); } @@ -3926,7 +3940,18 @@ function startLoadAllRows(segment: { offset: number; limit: number }) { function finishOrContinueLoadAllRun(requestedOffset: number | undefined, requestedLimit: number | undefined): boolean { if (!loadAllRowsLoopActive) return false; const effectiveTotal = paginationTotalRowCount.value ?? displayedTotalRowCount.value; - const canFetchMore = canFetchNextInfiniteScrollSegment.value && (!totalRowCountIsExact.value || effectiveTotal === undefined || props.result.rows.length < effectiveTotal); + const canFetchMore = canStartDataGridLoadAll({ + hasMore: props.result.has_more, + loadedRowCount: props.result.rows.length, + pageSize: pageSize.value, + totalRowCount: hasKnownPaginationTotalRowCount.value ? paginationTotalRowCount.value : undefined, + exactTotal: totalRowCountIsExact.value ? effectiveTotal : undefined, + allRowsLoaded: infiniteScrollAllLoaded, + localResultComplete: allRowsLoaded.value, + // The marker also gets set by the result-row cap; a capped result is not + // exhausted, and load-all is the only way past the cap (#10752). + resultRowCap: infiniteScrollMaxRows.value, + }); const nextSegment = loadsSqlPages.value ? nextLoadAllSegment() : canFetchMore diff --git a/apps/desktop/src/lib/__tests__/dataGrid/dataGridInfiniteScroll.spec.ts b/apps/desktop/src/lib/__tests__/dataGrid/dataGridInfiniteScroll.spec.ts index 375943e876..1e9d6a665e 100644 --- a/apps/desktop/src/lib/__tests__/dataGrid/dataGridInfiniteScroll.spec.ts +++ b/apps/desktop/src/lib/__tests__/dataGrid/dataGridInfiniteScroll.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { dataGridBottomScrollTop, dataGridInfiniteScrollAppendCompletion, didDataGridInfiniteScrollContextChange, isDataGridAtScrollBottom, restoredDataGridScrollLeft } from "@/lib/dataGrid/dataGridInfiniteScroll"; -import { dataGridLoadAllInitialTarget, dataGridLoadAllNextSegment } from "@/lib/dataGrid/dataGridPagination"; +import { canStartDataGridLoadAll, dataGridLoadAllInitialTarget, dataGridLoadAllNextSegment } from "@/lib/dataGrid/dataGridPagination"; describe("data grid bottom anchoring", () => { it("keeps DOM rows anchored when scrollbar padding increases the scroll height", () => { @@ -164,3 +164,48 @@ describe("dataGridLoadAllInitialTarget", () => { expect(dataGridLoadAllInitialTarget(200_000, 100_000)).toBe(300_000); }); }); + +describe("canStartDataGridLoadAll", () => { + const base = { loadedRowCount: 10, pageSize: 10, allRowsLoaded: false, localResultComplete: false }; + + it("blocks a re-request on an exhausted result even when the total is unknown (#11321)", () => { + // Fully loaded 10-row result (the backend's has_more stays true because the + // first page filled the page size), marker set by the short previous chunk. + expect(canStartDataGridLoadAll({ ...base, hasMore: true, allRowsLoaded: true })).toBe(false); + }); + + it("blocks a limit-free result that already holds every row", () => { + // A result fetched without a page limit cannot yield more rows; paging it + // must never re-request, even when the row count is below the page size. + expect(canStartDataGridLoadAll({ loadedRowCount: 5, pageSize: 100, allRowsLoaded: false, localResultComplete: true })).toBe(false); + expect(canStartDataGridLoadAll({ loadedRowCount: 5, pageSize: 100, hasMore: true, allRowsLoaded: false, localResultComplete: true })).toBe(false); + }); + + it("still starts when the marker was set by the result-row cap (#10752)", () => { + // The load watcher also sets the marker on a cap hit — a capped result is + // not exhausted, and load-all is the only way past the cap. has_more is + // still true and no exact total exists, so the run must start; a genuinely + // exhausted capped result costs one empty append before terminating. + expect(canStartDataGridLoadAll({ loadedRowCount: 100_000, pageSize: 100, hasMore: true, allRowsLoaded: true, localResultComplete: false, resultRowCap: 100_000 })).toBe(true); + // Below the cap the marker still blocks (#11321 shape). + expect(canStartDataGridLoadAll({ loadedRowCount: 10, pageSize: 10, hasMore: true, allRowsLoaded: true, localResultComplete: false, resultRowCap: 100_000 })).toBe(false); + }); + + it("blocks when the exact total has already been loaded", () => { + expect(canStartDataGridLoadAll({ ...base, hasMore: true, exactTotal: 10 })).toBe(false); + expect(canStartDataGridLoadAll({ ...base, hasMore: undefined, exactTotal: 10 })).toBe(false); + }); + + it("keeps issuing requests while rows may remain", () => { + expect(canStartDataGridLoadAll({ ...base, hasMore: true })).toBe(true); + expect(canStartDataGridLoadAll({ ...base, hasMore: true, exactTotal: 260_000 })).toBe(true); + expect(canStartDataGridLoadAll({ loadedRowCount: 100_000, pageSize: 100, hasMore: true, exactTotal: 260_000, allRowsLoaded: false, localResultComplete: false })).toBe(true); + }); + + it("falls back to the shared segment heuristic when nothing is known to be complete", () => { + // No has_more, no total, loaded below pageSize: nothing suggests more rows. + expect(canStartDataGridLoadAll({ loadedRowCount: 10, pageSize: 100, allRowsLoaded: false, localResultComplete: false })).toBe(false); + // Either completion marker short-circuits before any heuristic can say yes. + expect(canStartDataGridLoadAll({ loadedRowCount: 10, pageSize: 100, hasMore: true, allRowsLoaded: true, localResultComplete: false })).toBe(false); + }); +}); diff --git a/apps/desktop/src/lib/dataGrid/dataGridPagination.ts b/apps/desktop/src/lib/dataGrid/dataGridPagination.ts index bd687db0f9..51b1708bd7 100644 --- a/apps/desktop/src/lib/dataGrid/dataGridPagination.ts +++ b/apps/desktop/src/lib/dataGrid/dataGridPagination.ts @@ -205,3 +205,39 @@ export function dataGridLoadAllNextSegment(options: { loadedRowCount: number; re if (totalRowCount !== undefined && loadedRowCount >= totalRowCount) return null; return { offset: loadedRowCount, limit: totalRowCount !== undefined ? Math.min(requestedLimit, totalRowCount - loadedRowCount) : requestedLimit }; } + +/** + * Whether an explicit "load all" entry point may issue an append request at + * all. Everything already known to be complete must short-circuit to the + * reveal-only path: re-requesting against an exhausted result made the grid + * append whatever the backend answered with, duplicating rows on engines + * whose cursor or pagination layer does not honor the requested offset + * (#11321: clicking load-all a second time doubled a fully loaded 10-row + * result). Two independent completion signals are gated: + * - `allRowsLoaded` — the grid's own "no more rows" marker, set when a chunk + * came back short; must gate the run even when the total is unknown; + * - `localResultComplete` — a result fetched without a row limit already + * holds every row the backend will return; paging it cannot yield more. + * `loadedRowCount >= exactTotal` covers totals learned after either marker. + * + * The marker is ambiguous, though: the load watcher and the non-loadAll + * append-completion path also set it when the result-row CAP is hit — and a + * capped result is not exhausted. Load-all is the only way to fetch the rest + * (#10752), so `resultRowCap` releases the marker when the run sits at or + * past the cap. Worst case for a genuinely exhausted capped result is one + * empty append before the loop terminates. + */ +export function canStartDataGridLoadAll(options: { hasMore?: boolean; loadedRowCount: number; pageSize: number; totalRowCount?: number; exactTotal?: number; allRowsLoaded: boolean; localResultComplete: boolean; resultRowCap?: number }): boolean { + const loadedRowCount = Number.isFinite(options.loadedRowCount) ? Math.max(0, Math.trunc(options.loadedRowCount)) : 0; + const resultRowCap = typeof options.resultRowCap === "number" && Number.isFinite(options.resultRowCap) && options.resultRowCap > 0 ? Math.trunc(options.resultRowCap) : undefined; + const atResultRowCap = resultRowCap !== undefined && loadedRowCount >= resultRowCap; + if ((options.allRowsLoaded && !atResultRowCap) || options.localResultComplete) return false; + if (typeof options.exactTotal === "number" && Number.isFinite(options.exactTotal) && options.exactTotal >= 0 && loadedRowCount >= options.exactTotal) return false; + return canFetchNextDataGridSegment({ + hasMore: options.hasMore, + loadedRowCount: options.loadedRowCount, + pageSize: options.pageSize, + totalRowCount: options.totalRowCount, + allRowsLoaded: options.localResultComplete, + }); +}