mirror of
https://github.com/t8y2/dbx.git
synced 2026-10-20 00:17:44 +00:00
fix(grid): stop load-all re-requesting exhausted result sets
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user