From 81e23ab870fa6599b1348b52e4c53b1078246085 Mon Sep 17 00:00:00 2001 From: zipg Date: Tue, 21 Jul 2026 21:31:17 +0800 Subject: [PATCH] fix(export): preserve local query result constraints --- apps/desktop/src/components/grid/DataGrid.vue | 19 ++- .../src/composables/useDataGridExport.ts | 18 ++- .../src/lib/dataGrid/dataGridPagination.ts | 22 ++++ packages/app-tests/dataGridPagination.test.ts | 32 +++++- packages/app-tests/useDataGridExport.test.ts | 108 ++++++++++++++++++ 5 files changed, 192 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/components/grid/DataGrid.vue b/apps/desktop/src/components/grid/DataGrid.vue index bb50e47b17..225b904328 100644 --- a/apps/desktop/src/components/grid/DataGrid.vue +++ b/apps/desktop/src/components/grid/DataGrid.vue @@ -129,7 +129,7 @@ import { applyColumnFormatter, buildColumnFormatterKey, getSupportedTimeZoneOpti import { temporalCellEditorConfig, type TemporalCellEditorConfig } from "@/lib/dataGrid/dataGridTemporalEditor"; import { isCancelSearchShortcut, isCopyCurrentRowShortcut, isDeleteCurrentRowShortcut, isFocusSearchShortcut, isModRShortcut, isSaveShortcut, isToggleTransposeShortcut } from "@/lib/editor/keyboardShortcuts"; import { dataGridHeaderContentWidth, scrollbarGutterWidth } from "@/lib/dataGrid/dataGridScrollGutter"; -import { canGoNextDataGridPage } from "@/lib/dataGrid/dataGridPagination"; +import { canGoNextDataGridPage, hasCompleteLocalDataGridResult } from "@/lib/dataGrid/dataGridPagination"; import { dataGridCountQueryOptions } from "@/lib/dataGrid/dataGridQueryOptions"; import { dataGridBottomScrollTop, dataGridScrollPosition, isDataGridAtScrollBottom, isDataGridNearScrollBottom, shouldCheckInfiniteScrollAfterScroll, type DataGridScrollPosition } from "@/lib/dataGrid/dataGridInfiniteScroll"; import { CANVAS_DATA_GRID_ROW_HEIGHT, dataGridSearchMatchKey, drawCanvasDataGrid } from "@/lib/dataGrid/canvasDataGridRenderer"; @@ -2359,10 +2359,19 @@ const hasKnownTotalRowCount = computed(() => typeof serverKnownTotalRowCount.val // rowCount IS the total. Without this hint, the "page is full → assume more" // fallback in canGoNextDataGridPage lets the user keep clicking next forever. const allRowsLoaded = computed(() => isResultsContext.value && props.pageLimit === undefined); -// True when the in-memory result already holds the complete result set (results -// context, no server-side pagination, not truncated, no further pages). Used to -// skip re-executing the query on export and instead write the local rows. -const hasCompleteLocalResult = computed(() => !!props.result && allRowsLoaded.value && props.result.truncated !== true && props.result.has_more !== true); +// Skip re-executing a query when the first page already contains every row. +// This also covers paginated queries whose first page is shorter than the limit. +const hasCompleteLocalResult = computed(() => + hasCompleteLocalDataGridResult({ + isResultsContext: isResultsContext.value, + rowCount: props.result.rows.length, + pageLimit: props.pageLimit, + pageOffset: props.pageOffset, + totalRowCount: serverKnownTotalRowCount.value, + truncated: props.result.truncated, + hasMore: props.result.has_more, + }), +); const canGoNextPage = computed(() => { return canGoNextDataGridPage({ hasMore: props.result.has_more, diff --git a/apps/desktop/src/composables/useDataGridExport.ts b/apps/desktop/src/composables/useDataGridExport.ts index e3400b4c1c..d60e73939e 100644 --- a/apps/desktop/src/composables/useDataGridExport.ts +++ b/apps/desktop/src/composables/useDataGridExport.ts @@ -234,6 +234,22 @@ export function useDataGridExport(options: UseDataGridExportOptions) { return pattern ? { ...result, rows: formatTemporalRowsForExport(result.rows, result.columnTypes, pattern) } : result; } + function normalizeCompleteLocalResult(result: QueryResult): { columns: string[]; columnTypes: string[]; rows: CellValue[][] } { + const hiddenColumnIndexes = new Set(result.hidden_column_indexes ?? []); + const exportedColumnIndexes = result.columns.map((_, index) => index).filter((index) => !hiddenColumnIndexes.has(index)); + const hasHiddenColumns = exportedColumnIndexes.length !== result.columns.length; + const editorSettings = useSettingsStore().editorSettings; + const rows = editorSettings.exportRowLimitEnabled ? result.rows.slice(0, editorSettings.exportRowLimit) : result.rows; + + // Internal key columns are query-only metadata. Keep every user column, + // including columns hidden manually in the grid, while preserving alignment. + return { + columns: hasHiddenColumns ? exportedColumnIndexes.map((index) => result.columns[index]!) : result.columns, + columnTypes: hasHiddenColumns ? exportedColumnIndexes.map((index) => result.column_types?.[index] ?? "") : (result.column_types ?? []), + rows: hasHiddenColumns ? rows.map((row) => exportedColumnIndexes.map((index) => row[index])) : rows, + }; + } + async function resultToExport(rowIds?: number[], onProgress?: (info: { rowsExported: number; totalRows: number | null }) => void, useFullExport = true, formatDateTime = true): Promise<{ columns: string[]; columnTypes: string[]; rows: CellValue[][] }> { if (useFullExport && rowIds === undefined && fullExportResult && !hasCompleteLocalResult?.value) { const result = await fullExportResult(onProgress); @@ -245,7 +261,7 @@ export function useDataGridExport(options: UseDataGridExportOptions) { // and reflects client-side filters/search and unsaved edits, which would // silently change what the export contains. if (useFullExport && rowIds === undefined && hasCompleteLocalResult?.value && completeLocalResult?.value) { - return applyGlobalDateTimeExportFormat({ columns: completeLocalResult.value.columns, columnTypes: completeLocalResult.value.column_types ?? [], rows: completeLocalResult.value.rows }, formatDateTime); + return applyGlobalDateTimeExportFormat(normalizeCompleteLocalResult(completeLocalResult.value), formatDateTime); } return applyGlobalDateTimeExportFormat( { diff --git a/apps/desktop/src/lib/dataGrid/dataGridPagination.ts b/apps/desktop/src/lib/dataGrid/dataGridPagination.ts index 8fac6c158f..f880a6467a 100644 --- a/apps/desktop/src/lib/dataGrid/dataGridPagination.ts +++ b/apps/desktop/src/lib/dataGrid/dataGridPagination.ts @@ -11,6 +11,28 @@ export interface CanGoNextDataGridPageOptions { allRowsLoaded?: boolean; } +export interface CompleteLocalDataGridResultOptions { + isResultsContext: boolean; + rowCount: number; + pageLimit?: number; + pageOffset?: number; + totalRowCount?: number; + truncated?: boolean; + hasMore?: boolean; +} + +export function hasCompleteLocalDataGridResult(options: CompleteLocalDataGridResultOptions): boolean { + if (!options.isResultsContext || options.truncated === true || options.hasMore === true) return false; + if (options.pageLimit === undefined) return true; + if ((options.pageOffset ?? 0) !== 0) return false; + + const pageLimit = Math.max(1, options.pageLimit); + if (options.rowCount < pageLimit) return true; + + const totalRowCount = options.totalRowCount; + return typeof totalRowCount === "number" && Number.isFinite(totalRowCount) && totalRowCount >= 0 && options.rowCount >= totalRowCount; +} + export function canGoNextDataGridPage(options: CanGoNextDataGridPageOptions): boolean { if (options.hasMore === true) return true; diff --git a/packages/app-tests/dataGridPagination.test.ts b/packages/app-tests/dataGridPagination.test.ts index ca54de39ea..5723e4c2b2 100644 --- a/packages/app-tests/dataGridPagination.test.ts +++ b/packages/app-tests/dataGridPagination.test.ts @@ -1,6 +1,36 @@ import { strict as assert } from "node:assert"; import { test } from "vitest"; -import { canGoNextDataGridPage } from "../../apps/desktop/src/lib/dataGrid/dataGridPagination.ts"; +import { canGoNextDataGridPage, hasCompleteLocalDataGridResult } from "../../apps/desktop/src/lib/dataGrid/dataGridPagination.ts"; + +test("first query page is complete when its known total is already loaded", () => { + assert.equal( + hasCompleteLocalDataGridResult({ + isResultsContext: true, + rowCount: 2, + pageLimit: 500, + pageOffset: 0, + totalRowCount: 2, + truncated: false, + hasMore: false, + }), + true, + ); +}); + +test("local query result is incomplete when rows are truncated or start after the first page", () => { + const completeFirstPage = { + isResultsContext: true, + rowCount: 500, + pageLimit: 500, + pageOffset: 0, + totalRowCount: 500, + truncated: false, + hasMore: false, + }; + assert.equal(hasCompleteLocalDataGridResult({ ...completeFirstPage, truncated: true }), false); + assert.equal(hasCompleteLocalDataGridResult({ ...completeFirstPage, pageOffset: 500, totalRowCount: 1000 }), false); + assert.equal(hasCompleteLocalDataGridResult({ ...completeFirstPage, totalRowCount: undefined }), false); +}); test("known total disables next page at the last exact page", () => { assert.equal( diff --git a/packages/app-tests/useDataGridExport.test.ts b/packages/app-tests/useDataGridExport.test.ts index f6b3a31580..f20afcacaa 100644 --- a/packages/app-tests/useDataGridExport.test.ts +++ b/packages/app-tests/useDataGridExport.test.ts @@ -48,6 +48,29 @@ function installMemoryStorage() { }; } +function installTextDownloadCapture() { + let downloadedBlob: Blob | undefined; + const createObjectUrl = vi.spyOn(URL, "createObjectURL").mockImplementation((blob) => { + downloadedBlob = blob; + return "blob:test-export"; + }); + const revokeObjectUrl = vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => {}); + const originalDocument = Object.getOwnPropertyDescriptor(globalThis, "document"); + Object.defineProperty(globalThis, "document", { + configurable: true, + value: { createElement: () => ({ click: vi.fn(), href: "", download: "" }) }, + }); + return { + content: async () => downloadedBlob?.text(), + restore: () => { + createObjectUrl.mockRestore(); + revokeObjectUrl.mockRestore(); + if (originalDocument) Object.defineProperty(globalThis, "document", originalDocument); + else Reflect.deleteProperty(globalThis, "document"); + }, + }; +} + function buildExportHarness( options: { currentResultLabel?: string; @@ -56,6 +79,7 @@ function buildExportHarness( columnTypes?: Array; rows?: QueryResult["rows"]; allExportResults?: Array<{ sheetName: string; result: QueryResult; sql?: string }>; + completeLocalResult?: QueryResult; } = {}, ) { const exportColumns = options.columns ?? ["id", "name"]; @@ -124,6 +148,8 @@ function buildExportHarness( hasRowSelection: computed(() => false), fullExportResult, queryResultExportRequest, + hasCompleteLocalResult: computed(() => !!options.completeLocalResult), + completeLocalResult: computed(() => options.completeLocalResult), allExportResults: computed(() => options.allExportResults), currentResultLabel: computed(() => options.currentResultLabel), exportFileBaseName: computed(() => options.exportFileBaseName), @@ -668,6 +694,88 @@ test("full query result CSV export streams through the backend without loading a assert.equal(exportProgressState.value.filePath, apiMock.startQueryResultExport.mock.calls[0][0].filePath); }); +test("complete local query result XLSX export does not re-execute the query", async () => { + const completeLocalResult: QueryResult = { + columns: ["id", "name"], + column_types: ["int4", "text"], + rows: [ + [1, "Ada"], + [2, "Lin"], + ], + affected_rows: 0, + execution_time_ms: 1, + truncated: false, + has_more: false, + }; + const { composable, fullExportResult, queryResultExportRequest } = buildExportHarness({ completeLocalResult }); + + await composable.exportXlsx(); + + assert.equal(fullExportResult.mock.calls.length, 0); + assert.equal(queryResultExportRequest.mock.calls.length, 0); + assert.equal(apiMock.startQueryResultExport.mock.calls.length, 0); + assert.deepEqual(apiMock.exportQueryResultXlsx.mock.calls[0]?.slice(1), ["Export", ["id", "name"], ["int4", "text"], completeLocalResult.rows]); +}); + +test("complete local query result export removes only internal hidden columns", async () => { + const completeLocalResult: QueryResult = { + columns: ["id", "name", "__DBX_PK_id"], + column_types: ["int4", "text", "int4"], + rows: [ + [1, "Ada", 1], + [2, "Lin", 2], + ], + hidden_column_indexes: [2], + affected_rows: 0, + execution_time_ms: 1, + truncated: false, + has_more: false, + }; + const { composable } = buildExportHarness({ columns: ["id"], completeLocalResult }); + + await composable.exportXlsx(); + + assert.deepEqual(apiMock.exportQueryResultXlsx.mock.calls[0]?.slice(1), [ + "Export", + ["id", "name"], + ["int4", "text"], + [ + [1, "Ada"], + [2, "Lin"], + ], + ]); +}); + +test("complete local CSV, XLSX, and TXT exports honor the enabled row limit", async () => { + useSettingsStore().updateEditorSettings({ exportRowLimitEnabled: true, exportRowLimit: 100 }); + const completeLocalResult: QueryResult = { + columns: ["id"], + column_types: ["int4"], + rows: Array.from({ length: 101 }, (_, index) => [index + 1]), + affected_rows: 0, + execution_time_ms: 1, + truncated: false, + has_more: false, + }; + const { composable } = buildExportHarness({ completeLocalResult }); + + await composable.exportCsv(); + assert.equal(apiMock.exportQueryResultCsv.mock.calls[0]?.[2].length, 100); + + await composable.exportXlsx(); + assert.equal(apiMock.exportQueryResultXlsx.mock.calls[0]?.[4].length, 100); + + const download = installTextDownloadCapture(); + try { + await composable.exportTxt(); + const lines = (await download.content())?.split("\n"); + assert.equal(lines?.length, 101); + assert.equal(lines?.at(-1), "100"); + } finally { + download.restore(); + } +}); + test("full query result CSV export defaults to the saved SQL title", async () => { vi.useFakeTimers(); try {