diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.test.ts index 6283732dafc..8026fbfa630 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.test.ts @@ -79,7 +79,7 @@ function makeRow(id: string, position: number) { return { id, data: { name: `Row ${id}` }, position, executions: {} } } -function makePages(rowsPerPage: number[], totalCount: number) { +function makePages(rowsPerPage: number[], totalCount: number | null) { return rowsPerPage.map((count, pageIdx) => ({ rows: Array.from({ length: count }, (_, i) => makeRow(`r${pageIdx * 1000 + i}`, pageIdx * 1000 + i) @@ -90,6 +90,10 @@ function makePages(rowsPerPage: number[], totalCount: number) { const OK = { status: 'success', hasNextPage: false } as const +function makeHook(queryOptions = QUERY_OPTIONS) { + return useTable({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID, queryOptions }) +} + beforeEach(() => { capturedEffects.length = 0 vi.clearAllMocks() @@ -100,83 +104,71 @@ beforeEach(() => { describe('useTable – ensureAllRowsLoaded', () => { it('returns an empty array when cache is empty', async () => { mockGetQueryData.mockReturnValue(undefined) - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: QUERY_OPTIONS, - }) + const { ensureAllRowsLoaded } = makeHook() const rows = await ensureAllRowsLoaded() expect(rows).toEqual([]) expect(mockFetchNextPage).not.toHaveBeenCalled() }) - it('returns rows from cache immediately when last page is partial (< 1 page)', async () => { - const [page] = makePages([3], 3) - mockGetQueryData.mockReturnValue({ pages: [page] }) - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: QUERY_OPTIONS, - }) + it('returns cached rows without fetching when the count is covered by a partial page', async () => { + mockGetQueryData.mockReturnValue({ pages: makePages([3], 3) }) + const { ensureAllRowsLoaded } = makeHook() const rows = await ensureAllRowsLoaded() expect(rows).toHaveLength(3) expect(rows.map((r) => r.id)).toEqual(['r0', 'r1', 'r2']) - // Cache already complete — no HTTP request needed. expect(mockFetchNextPage).not.toHaveBeenCalled() }) - it('returns rows from cache immediately when last page is exactly one full page', async () => { - // A full page means getNextPageParam returns the next offset, so we must - // fetch once to confirm there is no page 2 (which returns 0 rows). After - // that empty page the last page is partial (0 < 1000) and the loop breaks. - const [page0] = makePages([1000], 1000) - const emptyPage = { rows: [], totalCount: 1000 } - mockGetQueryData - .mockReturnValueOnce({ pages: [page0] }) // loop iter 1: full → fetch - .mockReturnValueOnce({ pages: [page0, emptyPage] }) // loop iter 2: empty → break - .mockReturnValue({ pages: [page0, emptyPage] }) // final read - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: QUERY_OPTIONS, - }) + it('returns cached rows without fetching when the count is covered by exactly one full page', async () => { + // The totalCount fast-path terminates a covered drain without the + // empty-page confirmation request the old page-fullness heuristic needed. + mockGetQueryData.mockReturnValue({ pages: makePages([1000], 1000) }) + const { ensureAllRowsLoaded } = makeHook() const rows = await ensureAllRowsLoaded() expect(rows).toHaveLength(1000) - expect(rows[0].id).toBe('r0') - expect(rows[999].id).toBe('r999') + expect(mockFetchNextPage).not.toHaveBeenCalled() + }) + + it('keeps paging past a short page when the count says more rows exist', async () => { + // The regression this termination rule exists for: a page shorter than the + // requested size must not be read as end-of-table. + const [shortPage] = makePages([36], 100) + const rest = { + rows: Array.from({ length: 64 }, (_, i) => makeRow(`r${1000 + i}`, 1000 + i)), + totalCount: null, + } + mockGetQueryData + .mockReturnValueOnce({ pages: [shortPage] }) // iter 1 check: 36 < 100 → fetch + .mockReturnValueOnce({ pages: [shortPage, rest] }) // iter 1 progress: 2 > 1 + .mockReturnValue({ pages: [shortPage, rest] }) // iter 2 check: covered → break; final read + const { ensureAllRowsLoaded } = makeHook() + const rows = await ensureAllRowsLoaded() + expect(rows).toHaveLength(100) expect(mockFetchNextPage).toHaveBeenCalledTimes(1) }) - it('fetches one page when last cached page is full and there is more data', async () => { - const [page0, page1] = makePages([1000, 500], 1500) + it('drains until an empty page when the count is unknown', async () => { + const [page0] = makePages([1000], null) + const emptyPage = { rows: [], totalCount: null } mockGetQueryData - .mockReturnValueOnce({ pages: [page0] }) // loop iter 1: full → fetch - .mockReturnValueOnce({ pages: [page0, page1] }) // loop iter 2: partial → break - .mockReturnValue({ pages: [page0, page1] }) // final read - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: QUERY_OPTIONS, - }) + .mockReturnValueOnce({ pages: [page0] }) // iter 1 check: unknown count → fetch + .mockReturnValueOnce({ pages: [page0, emptyPage] }) // iter 1 progress: 2 > 1 + .mockReturnValue({ pages: [page0, emptyPage] }) // iter 2 check: empty page → break; final read + const { ensureAllRowsLoaded } = makeHook() const rows = await ensureAllRowsLoaded() - expect(rows).toHaveLength(1500) - expect(rows[0].id).toBe('r0') - expect(rows[1000].id).toBe('r1000') + expect(rows).toHaveLength(1000) expect(mockFetchNextPage).toHaveBeenCalledTimes(1) }) - it('fetches multiple pages for a large table until a partial page terminates the drain', async () => { + it('fetches multiple pages for a large table until the count is covered', async () => { const [page0, page1, page2] = makePages([1000, 1000, 500], 2500) mockGetQueryData - .mockReturnValueOnce({ pages: [page0] }) // iter 1: full → fetch - .mockReturnValueOnce({ pages: [page0, page1] }) // iter 2: full → fetch - .mockReturnValueOnce({ pages: [page0, page1, page2] }) // iter 3: partial → break - .mockReturnValue({ pages: [page0, page1, page2] }) // final read - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: QUERY_OPTIONS, - }) + .mockReturnValueOnce({ pages: [page0] }) // iter 1 check: 1000 < 2500 → fetch + .mockReturnValueOnce({ pages: [page0, page1] }) // iter 1 progress: 2 > 1 + .mockReturnValueOnce({ pages: [page0, page1] }) // iter 2 check: 2000 < 2500 → fetch + .mockReturnValueOnce({ pages: [page0, page1, page2] }) // iter 2 progress: 3 > 2 + .mockReturnValue({ pages: [page0, page1, page2] }) // iter 3 check: covered → break; final read + const { ensureAllRowsLoaded } = makeHook() const rows = await ensureAllRowsLoaded() expect(rows).toHaveLength(2500) expect(rows[0].id).toBe('r0') @@ -186,18 +178,21 @@ describe('useTable – ensureAllRowsLoaded', () => { }) it('throws when fetchNextPage returns an error status', async () => { - const [page0] = makePages([1000], 2000) - mockGetQueryData.mockReturnValue({ pages: [page0] }) + mockGetQueryData.mockReturnValue({ pages: makePages([1000], 2000) }) const error = new Error('Network failure') mockFetchNextPage.mockResolvedValueOnce({ status: 'error', error }) - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: QUERY_OPTIONS, - }) + const { ensureAllRowsLoaded } = makeHook() await expect(ensureAllRowsLoaded()).rejects.toThrow('Network failure') }) + it('throws when a fetch makes no progress instead of spinning', async () => { + // A cancelQueries race can resolve fetchNextPage without appending a page. + mockGetQueryData.mockReturnValue({ pages: makePages([1000], 2000) }) + const { ensureAllRowsLoaded } = makeHook() + await expect(ensureAllRowsLoaded()).rejects.toThrow('no progress') + expect(mockFetchNextPage).toHaveBeenCalledTimes(1) + }) + it('does not call fetchNextPage or getQueryData when workspaceId is empty', async () => { const { ensureAllRowsLoaded } = useTable({ workspaceId: '', @@ -224,15 +219,63 @@ describe('useTable – ensureAllRowsLoaded', () => { it('encodes queryOptions.filter into the queryKey passed to getQueryData', async () => { const filter = { column: 'name', operator: 'eq', value: 'Alice' } as never - const [page] = makePages([3], 3) - mockGetQueryData.mockReturnValue({ pages: [page] }) - const { ensureAllRowsLoaded } = useTable({ - workspaceId: WORKSPACE_ID, - tableId: TABLE_ID, - queryOptions: { filter, sort: null }, - }) + mockGetQueryData.mockReturnValue({ pages: makePages([3], 3) }) + const { ensureAllRowsLoaded } = makeHook({ filter, sort: null }) await ensureAllRowsLoaded() const queryKey = mockGetQueryData.mock.calls[0][0] as unknown[] expect(JSON.stringify(queryKey)).toContain('Alice') }) }) + +describe('useTable – ensureRowsLoadedUpTo', () => { + it('returns the first maxRows with hasMore when the cache already exceeds the cap', async () => { + mockGetQueryData.mockReturnValue({ pages: makePages([1000, 1000], 2000) }) + const { ensureRowsLoadedUpTo } = makeHook() + const result = await ensureRowsLoadedUpTo(1500) + expect(result.rows).toHaveLength(1500) + expect(result.hasMore).toBe(true) + expect(mockFetchNextPage).not.toHaveBeenCalled() + }) + + it('returns everything with hasMore false when the table fits under the cap', async () => { + mockGetQueryData.mockReturnValue({ pages: makePages([3], 3) }) + const { ensureRowsLoadedUpTo } = makeHook() + const result = await ensureRowsLoadedUpTo(50) + expect(result.rows).toHaveLength(3) + expect(result.hasMore).toBe(false) + expect(mockFetchNextPage).not.toHaveBeenCalled() + }) + + it('loads one row past the cap to make hasMore exact at the boundary', async () => { + const [page0, page1] = makePages([1000, 1000], 2000) + mockGetQueryData + .mockReturnValueOnce({ pages: [page0] }) // check: at cap but more exist → fetch + .mockReturnValueOnce({ pages: [page0, page1] }) // progress: 2 > 1 + .mockReturnValue({ pages: [page0, page1] }) // check: past cap → break; final read + const { ensureRowsLoadedUpTo } = makeHook() + const result = await ensureRowsLoadedUpTo(1000) + expect(result.rows).toHaveLength(1000) + expect(result.hasMore).toBe(true) + expect(mockFetchNextPage).toHaveBeenCalledTimes(1) + }) + + it('skips the boundary probe when the count is already covered', async () => { + mockGetQueryData.mockReturnValue({ pages: makePages([1000], 1000) }) + const { ensureRowsLoadedUpTo } = makeHook() + const result = await ensureRowsLoadedUpTo(1000) + expect(result.rows).toHaveLength(1000) + expect(result.hasMore).toBe(false) + expect(mockFetchNextPage).not.toHaveBeenCalled() + }) + + it('returns empty with hasMore false when ids are missing', async () => { + const { ensureRowsLoadedUpTo } = useTable({ + workspaceId: '', + tableId: TABLE_ID, + queryOptions: QUERY_OPTIONS, + }) + const result = await ensureRowsLoadedUpTo(10) + expect(result).toEqual({ rows: [], hasMore: false }) + expect(mockFetchNextPage).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts index 818cdf43d20..6995f7819e8 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts @@ -11,6 +11,7 @@ import { useInfiniteTableRows, useTable as useTableQuery, } from '@/hooks/queries/tables' +import { countLoadedTableRows, hasMoreTableRows } from '@/hooks/queries/utils/table-rows-pagination' import { useWorkflowStates, useWorkflows } from '@/hooks/queries/workflows' import type { WorkflowMetadata } from '@/stores/workflows/registry/types' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -124,13 +125,18 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams) // getQueryData bypasses React's render cycle — pages added by fetchNextPage // are visible synchronously after each await without waiting for a re-render. while (true) { - const data = queryClient.getQueryData(opts.queryKey) - const lastPage = data?.pages[data.pages.length - 1] - if (!lastPage || lastPage.rows.length < TABLE_LIMITS.MAX_QUERY_LIMIT) break + const pages = queryClient.getQueryData(opts.queryKey)?.pages ?? [] + if (!hasMoreTableRows(pages)) break const result = await fetchNextPage() if (result.status === 'error') { throw result.error ?? new Error('Failed to load table rows') } + const after = queryClient.getQueryData(opts.queryKey)?.pages.length ?? 0 + if (after <= pages.length) { + // A cancelQueries race (optimistic mutation) can resolve the fetch without + // appending a page; retrying would spin forever on the same state. + throw new Error('Table rows pagination made no progress') + } } return queryClient.getQueryData(opts.queryKey)?.pages.flatMap((p) => p.rows) ?? [] @@ -148,25 +154,26 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams) sort: queryOptions.sort, }) - // Load one past the cap so `hasMore` is exact: a full final page only - // *might* have a successor, so we confirm by loading row `maxRows + 1` - // rather than inferring truncation from page fullness. + // Load one past the cap when needed so `hasMore` is exact: with the cap + // covered but hasMoreTableRows still true, row `maxRows + 1` confirms it. while (true) { - const data = queryClient.getQueryData(opts.queryKey) - const loaded = data?.pages.reduce((sum, p) => sum + p.rows.length, 0) ?? 0 - if (loaded > maxRows) break - const lastPage = data?.pages[data.pages.length - 1] - if (!lastPage || lastPage.rows.length < TABLE_LIMITS.MAX_QUERY_LIMIT) break + const pages = queryClient.getQueryData(opts.queryKey)?.pages ?? [] + if (countLoadedTableRows(pages) > maxRows || !hasMoreTableRows(pages)) break const result = await fetchNextPage() if (result.status === 'error') { throw result.error ?? new Error('Failed to load table rows') } + const after = queryClient.getQueryData(opts.queryKey)?.pages.length ?? 0 + if (after <= pages.length) { + throw new Error('Table rows pagination made no progress') + } } - const all = queryClient.getQueryData(opts.queryKey)?.pages.flatMap((p) => p.rows) ?? [] + const pages = queryClient.getQueryData(opts.queryKey)?.pages ?? [] + const all = pages.flatMap((p) => p.rows) return { rows: all.length > maxRows ? all.slice(0, maxRows) : all, - hasMore: all.length > maxRows, + hasMore: all.length > maxRows || hasMoreTableRows(pages), } }, [workspaceId, tableId, queryOptions.filter, queryOptions.sort, queryClient, fetchNextPage] diff --git a/apps/sim/hooks/queries/tables.test.ts b/apps/sim/hooks/queries/tables.test.ts index de552d8bf03..e3ed602c1c1 100644 --- a/apps/sim/hooks/queries/tables.test.ts +++ b/apps/sim/hooks/queries/tables.test.ts @@ -348,6 +348,11 @@ describe('tableRowsParamsKey', () => { describe('tableRowsInfiniteOptions', () => { const PAGE_SIZE = 1000 + interface PageFixture { + rows: Array<{ id: string; orderKey?: string }> + totalCount: number | null + } + function makeOpts(pageSize = PAGE_SIZE, sort: unknown = null) { return tableRowsInfiniteOptions({ workspaceId: WORKSPACE_ID, @@ -358,47 +363,73 @@ describe('tableRowsInfiniteOptions', () => { }) as { queryKey: readonly unknown[] getNextPageParam: ( - lastPage: { rows: unknown[] }, - allPages: unknown[], + lastPage: PageFixture, + allPages: PageFixture[], lastPageParam: unknown ) => number | { orderKey: string; id: string } | undefined } } - it('getNextPageParam returns undefined for a partial page (drain terminates)', () => { + function makePage(count: number, totalCount: number | null, startAt = 0, withOrderKey = false) { + return { + rows: Array.from({ length: count }, (_, i) => ({ + id: `r${startAt + i}`, + ...(withOrderKey ? { orderKey: `a${startAt + i}` } : {}), + })), + totalCount, + } + } + + function next( + opts: ReturnType, + pages: PageFixture[], + lastPageParam: unknown = 0 + ) { + return opts.getNextPageParam(pages[pages.length - 1], pages, lastPageParam) + } + + it('getNextPageParam terminates when the count is covered by a partial page', () => { const opts = makeOpts() - const lastPage = { rows: Array.from({ length: 500 }, (_, i) => ({ id: `r${i}` })) } - expect(opts.getNextPageParam(lastPage, [], 0)).toBeUndefined() + expect(next(opts, [makePage(500, 500)])).toBeUndefined() }) - it('getNextPageParam returns undefined for an empty page', () => { + it('getNextPageParam terminates on an empty page', () => { const opts = makeOpts() - expect(opts.getNextPageParam({ rows: [] }, [], 0)).toBeUndefined() + expect(next(opts, [makePage(1000, null), makePage(0, null, 1000)])).toBeUndefined() }) - it('getNextPageParam returns next offset for a full page', () => { + it('getNextPageParam continues past a short page when the count says more rows exist', () => { + // The regression the termination rule exists for: a page shorter than the + // requested size (e.g. a byte-cut page) must not be read as end-of-table. const opts = makeOpts() - const fullPage = { rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}` })) } - expect(opts.getNextPageParam(fullPage, [], 0)).toBe(PAGE_SIZE) - expect(opts.getNextPageParam(fullPage, [], PAGE_SIZE)).toBe(PAGE_SIZE * 2) + expect(next(opts, [makePage(36, 100)])).toBe(36) }) - it('getNextPageParam advances correctly across three pages of 1000', () => { + it('getNextPageParam terminates a full page when the count is covered', () => { const opts = makeOpts() - const fullPage = { rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}` })) } - const lastPartialPage = { rows: Array.from({ length: 200 }, (_, i) => ({ id: `r${i}` })) } + expect(next(opts, [makePage(PAGE_SIZE, PAGE_SIZE)])).toBeUndefined() + }) - expect(opts.getNextPageParam(fullPage, [], 0)).toBe(1000) - expect(opts.getNextPageParam(fullPage, [], 1000)).toBe(2000) - expect(opts.getNextPageParam(lastPartialPage, [], 2000)).toBeUndefined() + it('getNextPageParam returns next offset for a full page with an unknown count', () => { + const opts = makeOpts() + expect(next(opts, [makePage(PAGE_SIZE, null)])).toBe(PAGE_SIZE) + }) + + it('getNextPageParam advances correctly across three pages', () => { + const opts = makeOpts() + const p0 = makePage(PAGE_SIZE, 2200) + const p1 = makePage(PAGE_SIZE, null, 1000) + const p2 = makePage(200, null, 2000) + + expect(next(opts, [p0])).toBe(1000) + expect(next(opts, [p0, p1], 1000)).toBe(2000) + expect(next(opts, [p0, p1, p2], 2000)).toBeUndefined() }) it('getNextPageParam returns a keyset cursor when rows carry orderKey and there is no sort', () => { const opts = makeOpts() - const fullPage = { - rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}`, orderKey: `a${i}` })), - } - expect(opts.getNextPageParam(fullPage, [], 0)).toEqual({ + const pages = [makePage(PAGE_SIZE, 2000, 0, true)] + expect(next(opts, pages)).toEqual({ orderKey: `a${PAGE_SIZE - 1}`, id: `r${PAGE_SIZE - 1}`, }) @@ -406,11 +437,10 @@ describe('tableRowsInfiniteOptions', () => { it('getNextPageParam falls back to offset for sorted views even with orderKey present', () => { const opts = makeOpts(PAGE_SIZE, { column: 'name', direction: 'asc' }) - const fullPage = { - rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}`, orderKey: `a${i}` })), - } - expect(opts.getNextPageParam(fullPage, [], 0)).toBe(PAGE_SIZE) - expect(opts.getNextPageParam(fullPage, [], PAGE_SIZE)).toBe(PAGE_SIZE * 2) + const p0 = makePage(PAGE_SIZE, 3000, 0, true) + const p1 = makePage(PAGE_SIZE, null, 1000, true) + expect(next(opts, [p0])).toBe(PAGE_SIZE) + expect(next(opts, [p0, p1], PAGE_SIZE)).toBe(PAGE_SIZE * 2) }) it('queryKey includes the result of tableRowsParamsKey', () => { diff --git a/apps/sim/hooks/queries/tables.ts b/apps/sim/hooks/queries/tables.ts index b59f0383170..72878e36039 100644 --- a/apps/sim/hooks/queries/tables.ts +++ b/apps/sim/hooks/queries/tables.ts @@ -16,7 +16,7 @@ import { useQueryClient, } from '@tanstack/react-query' import { useRouter } from 'next/navigation' -import { isValidationError } from '@/lib/api/client/errors' +import { extractValidationIssues, isValidationError } from '@/lib/api/client/errors' import { requestJson } from '@/lib/api/client/request' import type { ContractJsonResponse } from '@/lib/api/contracts' import { @@ -83,7 +83,6 @@ import type { TableDefinition, TableMetadata, TableRow, - TableRowsCursor, WorkflowGroup, WorkflowGroupDependencies, WorkflowGroupOutput, @@ -97,6 +96,10 @@ import { } from '@/lib/table/deps' import { runUploadStrategy } from '@/lib/uploads/client/direct-upload' import { type TableQueryScope, tableKeys } from '@/hooks/queries/utils/table-keys' +import { + getNextTableRowsPageParam, + type TableRowsPageParam, +} from '@/hooks/queries/utils/table-rows-pagination' const logger = createLogger('TableQueries') @@ -106,12 +109,6 @@ type TableRowsParams = Omit & sort?: Sort | null } -/** - * Infinite-rows page param: a keyset cursor on the default `(order_key, id)` order, or a numeric - * offset for sorted views / legacy rows without an order key. `0` doubles as the first page. - */ -export type TableRowsPageParam = number | TableRowsCursor - export type TableRowsResponse = Pick< ContractJsonResponse['data'], 'rows' | 'totalCount' @@ -476,18 +473,13 @@ export function tableRowsInfiniteOptions({ }) }, initialPageParam: 0 as TableRowsPageParam, - getNextPageParam: (lastPage, _allPages, lastPageParam): TableRowsPageParam | undefined => { - if (lastPage.rows.length < pageSize) return undefined - // Default order pages by keyset cursor — each page is an index seek on (order_key, id), - // where OFFSET would re-scan every prior row (O(N²) across a deep scroll / full drain). - // Sorted views (and legacy rows without an order key) fall back to offset paging. - if (!sort) { - const last = lastPage.rows[lastPage.rows.length - 1] - if (last?.orderKey) return { orderKey: last.orderKey, id: last.id } - } - const param = lastPageParam as TableRowsPageParam - return (typeof param === 'number' ? param : 0) + lastPage.rows.length - }, + // Termination comes from hasMoreTableRows (empty page / totalCount covered) — never from + // rows.length < pageSize, so a short server page can't be misread as end-of-table. + // Default order pages by keyset cursor — each page is an index seek on (order_key, id), + // where OFFSET would re-scan every prior row (O(N²) across a deep scroll / full drain). + // Sorted views (and legacy rows without an order key) fall back to offset paging. + getNextPageParam: (_lastPage, allPages): TableRowsPageParam | undefined => + getNextTableRowsPageParam(allPages, Boolean(sort)), staleTime: 30 * 1000, }) } @@ -519,9 +511,10 @@ export function useCreateTable(workspaceId: string) { body: { ...params, workspaceId }, }) }, + // Unlike row writes, table naming has no inline validation surface — the + // issue message (e.g. the NAME_PATTERN rule) must reach the user as a toast. onError: (error) => { - if (isValidationError(error)) return - toast.error(error.message, { duration: 5000 }) + toast.error(extractValidationIssues(error)[0]?.message ?? error.message, { duration: 5000 }) }, onSettled: () => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) @@ -565,9 +558,10 @@ export function useRenameTable(workspaceId: string) { body: { workspaceId, name }, }) }, + // Inline rename reverts the field on failure with no message of its own, so + // the validation issue (e.g. the NAME_PATTERN rule) must surface as a toast. onError: (error) => { - if (isValidationError(error)) return - toast.error(error.message, { duration: 5000 }) + toast.error(extractValidationIssues(error)[0]?.message ?? error.message, { duration: 5000 }) }, onSettled: (_data, _error, variables) => { queryClient.invalidateQueries({ queryKey: tableKeys.detail(variables.tableId) }) diff --git a/apps/sim/hooks/queries/utils/table-rows-pagination.test.ts b/apps/sim/hooks/queries/utils/table-rows-pagination.test.ts new file mode 100644 index 00000000000..d34eee750de --- /dev/null +++ b/apps/sim/hooks/queries/utils/table-rows-pagination.test.ts @@ -0,0 +1,81 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + countLoadedTableRows, + getNextTableRowsPageParam, + hasMoreTableRows, +} from '@/hooks/queries/utils/table-rows-pagination' + +function makePage(count: number, totalCount: number | null, startAt = 0, withOrderKey = true) { + return { + rows: Array.from({ length: count }, (_, i) => ({ + id: `r${startAt + i}`, + ...(withOrderKey ? { orderKey: `k${String(startAt + i).padStart(6, '0')}` } : {}), + })), + totalCount, + } +} + +describe('countLoadedTableRows', () => { + it('sums rows across pages', () => { + expect(countLoadedTableRows([])).toBe(0) + expect(countLoadedTableRows([makePage(3, 10), makePage(2, null, 3)])).toBe(5) + }) +}) + +describe('hasMoreTableRows', () => { + it('returns false with no pages', () => { + expect(hasMoreTableRows([])).toBe(false) + }) + + it('returns false when the last page is empty', () => { + expect(hasMoreTableRows([makePage(1000, null), makePage(0, null, 1000)])).toBe(false) + }) + + it('returns false when the page-0 count is covered', () => { + expect(hasMoreTableRows([makePage(3, 3)])).toBe(false) + }) + + it('returns true for a short page when the count says more exist', () => { + // The regression this module exists for: a page shorter than the requested + // size must never be read as end-of-table on its own. + expect(hasMoreTableRows([makePage(36, 100)])).toBe(true) + }) + + it('returns true when the count is unknown and the last page is non-empty', () => { + expect(hasMoreTableRows([makePage(1000, null)])).toBe(true) + }) + + it('returns false when a stale-low count is already exceeded', () => { + expect(hasMoreTableRows([makePage(10, 5)])).toBe(false) + }) +}) + +describe('getNextTableRowsPageParam', () => { + it('returns undefined when no more rows exist', () => { + expect(getNextTableRowsPageParam([makePage(3, 3)], false)).toBeUndefined() + expect(getNextTableRowsPageParam([makePage(1000, null), makePage(0, null)], false)).toBe( + undefined + ) + }) + + it('returns the keyset cursor of the last loaded row on the default order', () => { + const pages = [makePage(1000, 2000), makePage(500, null, 1000)] + expect(getNextTableRowsPageParam(pages, false)).toEqual({ + orderKey: 'k001499', + id: 'r1499', + }) + }) + + it('returns the loaded-row offset for sorted views, even after short pages', () => { + const pages = [makePage(1000, 2000), makePage(36, null, 1000)] + expect(getNextTableRowsPageParam(pages, true)).toBe(1036) + }) + + it('falls back to the loaded-row offset when the last row has no order key', () => { + const pages = [makePage(700, 2000, 0, false)] + expect(getNextTableRowsPageParam(pages, false)).toBe(700) + }) +}) diff --git a/apps/sim/hooks/queries/utils/table-rows-pagination.ts b/apps/sim/hooks/queries/utils/table-rows-pagination.ts new file mode 100644 index 00000000000..afca1ea5c22 --- /dev/null +++ b/apps/sim/hooks/queries/utils/table-rows-pagination.ts @@ -0,0 +1,52 @@ +import type { TableRowsCursor } from '@/lib/table/types' + +/** + * Infinite-rows page param: a keyset cursor on the default `(order_key, id)` order, or a numeric + * offset for sorted views / legacy rows without an order key. `0` doubles as the first page. + */ +export type TableRowsPageParam = number | TableRowsCursor + +interface TableRowsPageLike { + rows: ReadonlyArray<{ id: string; orderKey?: string }> + totalCount: number | null +} + +/** Rows loaded across all fetched pages. */ +export function countLoadedTableRows(pages: readonly TableRowsPageLike[]): number { + return pages.reduce((sum, page) => sum + page.rows.length, 0) +} + +/** + * Whether more rows may exist past the fetched pages. A page is terminal only when it is + * empty or when page 0's `COUNT(*)` is already covered — never when it is merely shorter + * than the requested page size, so a short server page can never be misread as end-of-table. + * + * `totalCount` is advisory (computed in a separate transaction from the page read). A + * stale-high count self-corrects via the empty-page rule at the cost of one extra request; + * a stale-low count (rows deleted after page 0's COUNT) stops the drain early — accepted, + * since the view is already stale and the run-stream/interval invalidations refetch it. + */ +export function hasMoreTableRows(pages: readonly TableRowsPageLike[]): boolean { + const lastPage = pages[pages.length - 1] + if (!lastPage || lastPage.rows.length === 0) return false + const totalCount = pages[0].totalCount + return totalCount == null || countLoadedTableRows(pages) < totalCount +} + +/** + * Continuation for the next page: a keyset cursor from the last loaded row on the default + * order, else the absolute offset — the actual loaded-row count, not pages × pageSize, so + * short pages resume without gaps. + */ +export function getNextTableRowsPageParam( + pages: readonly TableRowsPageLike[], + sorted: boolean +): TableRowsPageParam | undefined { + if (!hasMoreTableRows(pages)) return undefined + const lastPage = pages[pages.length - 1] + if (!sorted) { + const last = lastPage.rows[lastPage.rows.length - 1] + if (last?.orderKey) return { orderKey: last.orderKey, id: last.id } + } + return countLoadedTableRows(pages) +} diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index fe224161b03..1c03ce965c8 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -96,6 +96,7 @@ export const env = createEnv({ ENTERPRISE_TABLES_LIMIT: z.number().optional(), // Max user tables per workspace on enterprise tier (default: 10000) ENTERPRISE_TABLE_ROWS_LIMIT: z.number().optional(), // Max rows per table on enterprise tier (default: 1000000) TABLE_MAX_ROW_SIZE_BYTES: z.number().optional(), // Max serialized size in bytes of a single user-table row (default: 409600) + TABLE_MAX_PAGE_BYTES: z.number().optional(), // Dev-preview: byte budget per row-page read; pages cut early past it (unset = disabled) // Credit-tier Stripe prices (monthly) STRIPE_PRICE_TIER_25_MO: z.string().min(1).optional(), // Pro: $25/mo (6,000 credits) diff --git a/apps/sim/lib/table/__tests__/paging.test.ts b/apps/sim/lib/table/__tests__/paging.test.ts new file mode 100644 index 00000000000..22a4a55bcf5 --- /dev/null +++ b/apps/sim/lib/table/__tests__/paging.test.ts @@ -0,0 +1,33 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { trimRowsToByteBudget } from '@/lib/table/rows/paging' + +function row(id: string, bytes: number) { + // {"b":"aaa…"} serializes to bytes + 8 chars of envelope. + return { id, data: { b: 'a'.repeat(Math.max(0, bytes - 8)) } } +} + +describe('trimRowsToByteBudget', () => { + it('returns all rows when they fit the budget', () => { + const rows = [row('r1', 100), row('r2', 100)] + expect(trimRowsToByteBudget(rows, 1000)).toBe(rows) + }) + + it('keeps the longest prefix within the budget', () => { + const rows = [row('r1', 400), row('r2', 400), row('r3', 400)] + const kept = trimRowsToByteBudget(rows, 900) + expect(kept.map((r) => r.id)).toEqual(['r1', 'r2']) + }) + + it('always keeps the first row even when it alone exceeds the budget', () => { + const rows = [row('r1', 5000), row('r2', 100)] + const kept = trimRowsToByteBudget(rows, 1000) + expect(kept.map((r) => r.id)).toEqual(['r1']) + }) + + it('returns empty input unchanged', () => { + expect(trimRowsToByteBudget([], 1000)).toEqual([]) + }) +}) diff --git a/apps/sim/lib/table/__tests__/validation.test.ts b/apps/sim/lib/table/__tests__/validation.test.ts index 4ebfe9a6ffa..40e3187270c 100644 --- a/apps/sim/lib/table/__tests__/validation.test.ts +++ b/apps/sim/lib/table/__tests__/validation.test.ts @@ -179,6 +179,14 @@ describe('Validation', () => { expect(result.valid).toBe(false) expect(result.errors[0]).toContain('exceeds limit') }) + + it('should measure UTF-8 bytes, not UTF-16 code units', () => { + // '工' is one UTF-16 code unit but three UTF-8 bytes — a char-count check + // would accept this row at ~1/3 of the real serialized size. + const chars = Math.ceil(TABLE_LIMITS.MAX_ROW_SIZE_BYTES / 3) + 1 + const result = validateRowSize({ content: '工'.repeat(chars) }) + expect(result.valid).toBe(false) + }) }) describe('validateRowAgainstSchema', () => { @@ -270,12 +278,11 @@ describe('Validation', () => { expect(result.valid).toBe(true) }) - it('should reject string exceeding max length', () => { - const longString = 'a'.repeat(TABLE_LIMITS.MAX_STRING_VALUE_LENGTH + 1) + it('should allow long strings — cell size is bounded by the row byte cap, not per-value', () => { + const longString = 'a'.repeat(100_000) const data = { name: longString } const result = validateRowAgainstSchema(data, schema) - expect(result.valid).toBe(false) - expect(result.errors[0]).toContain('exceeds max string length') + expect(result.valid).toBe(true) }) }) diff --git a/apps/sim/lib/table/constants.ts b/apps/sim/lib/table/constants.ts index e68340bbe44..4a275abfe54 100644 --- a/apps/sim/lib/table/constants.ts +++ b/apps/sim/lib/table/constants.ts @@ -12,7 +12,6 @@ export const TABLE_LIMITS = { MAX_COLUMNS_PER_TABLE: 50, MAX_TABLE_NAME_LENGTH: 128, MAX_COLUMN_NAME_LENGTH: 50, - MAX_STRING_VALUE_LENGTH: 10000, MAX_DESCRIPTION_LENGTH: 500, DEFAULT_QUERY_LIMIT: 100, MAX_QUERY_LIMIT: 1000, @@ -61,6 +60,17 @@ export const DEFAULT_TABLE_PLAN_LIMITS = { }, } as const +/** + * Byte budget for one page of row reads, or null when disabled (the default). + * Dev-preview of the byte-bounded pagination follow-up: set `TABLE_MAX_PAGE_BYTES` + * to cut pages early once their serialized row data exceeds the budget. The + * production version moves the cut into SQL — see the pagination-hardening plan. + */ +export function getMaxPageBytes(): number | null { + const value = envNumber(env.TABLE_MAX_PAGE_BYTES, 0, { min: 0, integer: true }) + return value > 0 ? value : null +} + /** * Maximum serialized size in bytes of a single row. Defaults to * `TABLE_LIMITS.MAX_ROW_SIZE_BYTES`; overridable via the diff --git a/apps/sim/lib/table/rows/paging.ts b/apps/sim/lib/table/rows/paging.ts new file mode 100644 index 00000000000..a96b98c041f --- /dev/null +++ b/apps/sim/lib/table/rows/paging.ts @@ -0,0 +1,23 @@ +/** + * Cuts a fetched page to a byte budget: keeps the longest prefix of rows whose + * serialized `data` fits within `maxBytes`, always keeping at least one row so + * pagination makes forward progress even when a single row exceeds the budget. + * + * The budget counts `data` only — the per-row envelope (`id`, `position`, + * `orderKey`, timestamps, executions) is not measured, so actual response + * payloads run slightly over `maxBytes`. Callers must leave headroom; the + * production SQL-side cut should account for the same overhead. + */ +export function trimRowsToByteBudget( + rows: T[], + maxBytes: number +): T[] { + let total = 0 + let kept = 0 + for (const row of rows) { + total += Buffer.byteLength(JSON.stringify(row.data)) + if (kept > 0 && total > maxBytes) break + kept++ + } + return kept === rows.length ? rows : rows.slice(0, kept) +} diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 988695e87b5..75497ad8aeb 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -25,7 +25,7 @@ import { wouldExceedRowLimit, } from '@/lib/table/billing' import { getColumnId } from '@/lib/table/column-keys' -import { TABLE_LIMITS, USER_TABLE_ROWS_SQL_NAME } from '@/lib/table/constants' +import { getMaxPageBytes, TABLE_LIMITS, USER_TABLE_ROWS_SQL_NAME } from '@/lib/table/constants' import { nKeysBetween } from '@/lib/table/order-key' import { type DbExecutor, type DbTransaction, withSeqscanOff } from '@/lib/table/planner' import { @@ -46,6 +46,7 @@ import { resolveBatchInsertOrderKeys, resolveInsertOrderKey, } from '@/lib/table/rows/ordering' +import { trimRowsToByteBudget } from '@/lib/table/rows/paging' import { buildFilterClause, buildSortClause, escapeLikePattern } from '@/lib/table/sql' import { fireTableTrigger } from '@/lib/table/trigger' import { scaledStatementTimeoutMs, setTableTxTimeouts } from '@/lib/table/tx' @@ -1028,7 +1029,12 @@ export async function queryRows( .then((r) => Number(r[0].count)) : null - const [rows, totalCount] = await Promise.all([rowsPromise, countPromise]) + const [fetchedRows, totalCount] = await Promise.all([rowsPromise, countPromise]) + + // Dev-preview byte cut (TABLE_MAX_PAGE_BYTES, off by default): clients terminate on + // empty page / totalCount, never page fullness, so a short page is safe to return. + const maxPageBytes = getMaxPageBytes() + const rows = maxPageBytes === null ? fetchedRows : trimRowsToByteBudget(fetchedRows, maxPageBytes) const executionsByRow = withExecutions ? await loadExecutionsByRow( diff --git a/apps/sim/lib/table/validation.ts b/apps/sim/lib/table/validation.ts index a3066f1e341..0029f576845 100644 --- a/apps/sim/lib/table/validation.ts +++ b/apps/sim/lib/table/validation.ts @@ -228,8 +228,6 @@ export function validateRowAgainstSchema(data: RowData, schema: TableSchema): Va case 'string': if (typeof value !== 'string') { errors.push(`${column.name} must be string, got ${typeof value}`) - } else if (value.length > TABLE_LIMITS.MAX_STRING_VALUE_LENGTH) { - errors.push(`${column.name} exceeds max string length`) } break case 'number': @@ -354,10 +352,10 @@ export function coerceRowToSchema(data: RowData, schema: TableSchema): Validatio return validateRowAgainstSchema(data, schema) } -/** Validates row data size is within limits. */ +/** Validates row data size (UTF-8 bytes of the serialized row) is within limits. */ export function validateRowSize(data: RowData): ValidationResult { const maxRowSizeBytes = getMaxRowSizeBytes() - const size = JSON.stringify(data).length + const size = Buffer.byteLength(JSON.stringify(data)) if (size > maxRowSizeBytes) { return { valid: false,