diff --git a/apps/sim/app/api/table/[tableId]/rows/route.ts b/apps/sim/app/api/table/[tableId]/rows/route.ts index ca239534696..e200220ea11 100644 --- a/apps/sim/app/api/table/[tableId]/rows/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/route.ts @@ -74,7 +74,6 @@ async function handleBatchInsert( rows, workspaceId: validated.workspaceId, userId, - positions: validated.positions, orderKeys: validated.orderKeys, }, table, diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 2fcbb8a0d1d..330df1c2d2d 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -1017,14 +1017,13 @@ export function TableGrid({ const anchorId = contextMenu.row.id // Fractional ordering: express intent by neighbor id, not integer position. const intent = offset === 0 ? { beforeRowId: anchorId } : { afterRowId: anchorId } - const position = contextMenu.row.position + offset createRef.current( { data: {}, ...intent }, { onSuccess: (response: Record) => { const newRowId = extractCreatedRowId(response) if (newRowId) { - pushUndoRef.current({ type: 'create-row', rowId: newRowId, position }) + pushUndoRef.current({ type: 'create-row', rowId: newRowId }) } }, } @@ -1096,7 +1095,6 @@ export function TableGrid({ const contextRow = contextMenu.row if (!contextRow) return const rowData = { ...contextRow.data } - const position = contextRow.position + 1 const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextRow.id) closeContextMenu() createRef.current( @@ -1108,7 +1106,6 @@ export function TableGrid({ pushUndoRef.current({ type: 'create-row', rowId: newRowId, - position, data: rowData, }) } @@ -1143,11 +1140,9 @@ export function TableGrid({ onSuccess: (response: Record) => { const newRowId = extractCreatedRowId(response) if (newRowId) { - const maxPosition = rowsRef.current.reduce((max, r) => Math.max(max, r.position), -1) pushUndoRef.current({ type: 'create-row', rowId: newRowId, - position: maxPosition + 1, }) } }, @@ -2187,7 +2182,7 @@ export function TableGrid({ onSuccess: (response: Record) => { const newRowId = extractCreatedRowId(response) if (newRowId) { - pushUndoRef.current({ type: 'create-row', rowId: newRowId, position }) + pushUndoRef.current({ type: 'create-row', rowId: newRowId }) } setSelectionAnchor({ rowIndex: anchor.rowIndex + 1, colIndex }) setSelectionFocus(null) @@ -2725,14 +2720,10 @@ export function TableGrid({ const currentCols = columnsRef.current const currentRows = rowsRef.current - // Captured once before the loop so each new row in the batch gets a unique, - // sequential position via `+ (newRowIndex - currentRows.length)` below. - const lastRowPosition = currentRows.reduce((max, r) => Math.max(max, r.position), -1) const undoCells: Array<{ rowId: string; data: Record }> = [] const updateBatch: Array<{ rowId: string; data: Record }> = [] const createBatchRows: Array> = [] - const createBatchPositions: number[] = [] for (let r = 0; r < pasteRows.length; r++) { const targetArrayIndex = currentAnchor.rowIndex + r @@ -2764,7 +2755,6 @@ export function TableGrid({ updateBatch.push({ rowId: existingRow.id, data: rowData }) } else { createBatchRows.push(rowData) - createBatchPositions.push(lastRowPosition + 1 + (targetArrayIndex - currentRows.length)) } } @@ -2782,20 +2772,18 @@ export function TableGrid({ if (createBatchRows.length > 0) { batchCreateRef.current( - { rows: createBatchRows, positions: createBatchPositions }, + { rows: createBatchRows }, { onSuccess: (response) => { const createdRows = response?.data?.rows ?? [] const undoRows: Array<{ rowId: string - position: number data: Record }> = [] for (let i = 0; i < createdRows.length; i++) { if (createdRows[i]?.id) { undoRows.push({ rowId: createdRows[i].id, - position: createBatchPositions[i], data: createBatchRows[i], }) } diff --git a/apps/sim/hooks/queries/tables.ts b/apps/sim/hooks/queries/tables.ts index a33f2e240c9..7510875ebbf 100644 --- a/apps/sim/hooks/queries/tables.ts +++ b/apps/sim/hooks/queries/tables.ts @@ -817,7 +817,7 @@ type BatchCreateTableRowsParams = Omit /** - * Batch create rows in a table. Supports optional per-row positions for undo restore. + * Batch create rows in a table. Supports optional per-row order keys for undo restore. */ export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationContext) { const queryClient = useQueryClient() @@ -832,7 +832,6 @@ export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationCon body: { workspaceId, rows: variables.rows as RowData[], - positions: variables.positions, orderKeys: variables.orderKeys, }, }) diff --git a/apps/sim/hooks/use-table-undo.ts b/apps/sim/hooks/use-table-undo.ts index 4014043c71e..dd403c4c27c 100644 --- a/apps/sim/hooks/use-table-undo.ts +++ b/apps/sim/hooks/use-table-undo.ts @@ -136,12 +136,11 @@ export function useTableUndo({ deleteRowMutation.mutate(action.rowId) } else { // Redo via the batch path so the saved orderKey restores exact placement. - // The single-insert API has no orderKey field, and under the fractional-ordering - // flag its `position` is read as a rank — a gappy saved position misplaces. + // The single-insert API has no orderKey field, and order_key is authoritative — + // a gappy saved position would misplace the row. batchCreateRowsMutation.mutate( { rows: [action.data ?? {}], - positions: [action.position], orderKeys: action.orderKey ? [action.orderKey] : undefined, }, { @@ -169,7 +168,6 @@ export function useTableUndo({ batchCreateRowsMutation.mutate( { rows: action.rows.map((r) => r.data), - positions: action.rows.map((r) => r.position), orderKeys: action.rows.every((r) => r.orderKey) ? action.rows.map((r) => r.orderKey as string) : undefined, @@ -194,7 +192,6 @@ export function useTableUndo({ batchCreateRowsMutation.mutate( { rows: action.rows.map((row) => row.data), - positions: action.rows.map((row) => row.position), orderKeys: action.rows.every((row) => row.orderKey) ? action.rows.map((row) => row.orderKey as string) : undefined, diff --git a/apps/sim/lib/api/contracts/tables.ts b/apps/sim/lib/api/contracts/tables.ts index d657619f710..c357cc585e8 100644 --- a/apps/sim/lib/api/contracts/tables.ts +++ b/apps/sim/lib/api/contracts/tables.ts @@ -199,16 +199,9 @@ export const batchInsertTableRowsBodySchema = z TABLE_LIMITS.MAX_BATCH_INSERT_SIZE, `Cannot insert more than ${TABLE_LIMITS.MAX_BATCH_INSERT_SIZE} rows per batch` ), - positions: z.array(z.number().int().min(0)).max(TABLE_LIMITS.MAX_BATCH_INSERT_SIZE).optional(), - /** Fractional ordering: exact per-row order keys (undo restore). Takes precedence over `positions`. */ + /** Fractional ordering: exact per-row order keys (undo restore). */ orderKeys: z.array(z.string().min(1)).max(TABLE_LIMITS.MAX_BATCH_INSERT_SIZE).optional(), }) - .refine((data) => !data.positions || data.positions.length === data.rows.length, { - message: 'positions array length must match rows array length', - }) - .refine((data) => !data.positions || new Set(data.positions).size === data.positions.length, { - message: 'positions must not contain duplicates', - }) .refine((data) => !data.orderKeys || data.orderKeys.length === data.rows.length, { message: 'orderKeys array length must match rows array length', }) diff --git a/apps/sim/lib/copilot/tools/server/table/user-table.ts b/apps/sim/lib/copilot/tools/server/table/user-table.ts index ef1a8fed1b2..9e24963856a 100644 --- a/apps/sim/lib/copilot/tools/server/table/user-table.ts +++ b/apps/sim/lib/copilot/tools/server/table/user-table.ts @@ -506,20 +506,6 @@ export const userTableServerTool: BaseServerTool return { success: false, message: 'Workspace ID is required' } } - const positions = args.positions as number[] | undefined - if (positions !== undefined && positions.length !== args.rows.length) { - return { - success: false, - message: `positions length (${positions.length}) must match rows length (${args.rows.length})`, - } - } - if (positions !== undefined && new Set(positions).size !== positions.length) { - return { - success: false, - message: 'positions must not contain duplicate values', - } - } - const table = await getTableById(args.tableId) if (!table || table.workspaceId !== workspaceId) { return { success: false, message: `Table not found: ${args.tableId}` } @@ -535,7 +521,6 @@ export const userTableServerTool: BaseServerTool rows: args.rows.map((r: RowData) => rowDataNameToId(r, idByName)), workspaceId, userId: context.userId, - positions, }, table, requestId diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index 1d992f2b03c..d7903b9a380 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -79,8 +79,7 @@ export const env = createEnv({ ENTERPRISE_TIER_COST_LIMIT: z.number().optional(), // Cost limit for enterprise tier users ENTERPRISE_STORAGE_LIMIT_GB: z.number().optional().default(500), // Default storage limit in GB for enterprise tier (can be overridden per org) BILLING_ENABLED: z.boolean().optional(), // Enable billing enforcement and usage tracking - FREE_API_DEPLOYMENT_GATE_ENABLED: z.boolean().optional(), // Block free-plan accounts from programmatic execution (API/MCP/generic webhooks/chat embeds). Requires BILLING_ENABLED. Off by default for dark rollout - TABLES_FRACTIONAL_ORDERING: z.boolean().optional(), // Order table rows by fractional order_key (O(1) insert/delete) instead of integer position + FREE_API_DEPLOYMENT_GATE_ENABLED: z.boolean().optional(), // Block free-plan accounts from programmatic execution (API/MCP/A2A/generic webhooks/chat embeds). Requires BILLING_ENABLED. Off by default for dark rollout TABLE_SNAPSHOT_CACHE: z.boolean().optional(), // Mount tables into sandboxes by reference via a version-keyed CSV snapshot in object storage instead of draining the whole table into web-process heap PII_REDACTION: z.boolean().optional(), // Redact PII from workflow logs via configurable Data Retention rules (Presidio at the logger persist choke point) and expose the Data Retention config UI PII_GRANULAR_REDACTION: z.boolean().optional(), // Expose the execution-altering PII redaction stages (redact workflow input + block outputs in-flight) in the Data Retention config; layered on top of PII_REDACTION diff --git a/apps/sim/lib/core/config/feature-flags.test.ts b/apps/sim/lib/core/config/feature-flags.test.ts index d9a251d472f..8c9b95a7556 100644 --- a/apps/sim/lib/core/config/feature-flags.test.ts +++ b/apps/sim/lib/core/config/feature-flags.test.ts @@ -46,9 +46,9 @@ function withAppConfig(doc: unknown) { } /** - * `isFeatureEnabled` only accepts registered `FeatureFlagName`s. The registry is - * empty in this PR, so tests reference flags through the AppConfig document and - * cast their throwaway names through this helper. + * `isFeatureEnabled` only accepts registered `FeatureFlagName`s. These tests + * exercise the evaluation logic with throwaway flag names supplied through the + * AppConfig document, cast to `FeatureFlagName` through this helper. */ const enabled = (flag: string, ctx?: FeatureFlagContext) => isFeatureEnabled(flag as FeatureFlagName, ctx) @@ -62,7 +62,6 @@ describe('getFeatureFlags', () => { it('derives flags from fallback secrets when AppConfig is disabled, without fetching', async () => { const flags = await getFeatureFlags() // All registered flags should be present, disabled (env vars unset in test env) - expect(flags['tables-fractional-ordering']).toEqual({ enabled: false }) expect(flags['mothership-beta']).toEqual({ enabled: false }) expect(flags['pii-redaction']).toEqual({ enabled: false }) expect(flags['pii-granular-redaction']).toEqual({ enabled: false }) @@ -91,7 +90,6 @@ describe('getFeatureFlags', () => { flagRef.isAppConfigEnabled = true mockFetch.mockResolvedValue(null) const flags = await getFeatureFlags() - expect(flags['tables-fractional-ordering']).toEqual({ enabled: false }) expect(flags['mothership-beta']).toEqual({ enabled: false }) expect(flags['pii-redaction']).toEqual({ enabled: false }) expect(flags['pii-granular-redaction']).toEqual({ enabled: false }) diff --git a/apps/sim/lib/core/config/feature-flags.ts b/apps/sim/lib/core/config/feature-flags.ts index 1f345632c6b..8a44624ba78 100644 --- a/apps/sim/lib/core/config/feature-flags.ts +++ b/apps/sim/lib/core/config/feature-flags.ts @@ -62,10 +62,6 @@ interface FeatureFlagDefinition { /** The single registry of known flags. To add a flag, add one entry here. */ const FEATURE_FLAGS = { - 'tables-fractional-ordering': { - description: 'Order table rows by fractional order_key instead of legacy integer position', - fallback: 'TABLES_FRACTIONAL_ORDERING', - }, 'mothership-beta': { description: 'Mothership beta plan/changelog artifact surfaces in the copilot VFS and doc compiler. ' + diff --git a/apps/sim/lib/table/__tests__/update-row.test.ts b/apps/sim/lib/table/__tests__/update-row.test.ts index b1135cd8d1b..bc5209d40f1 100644 --- a/apps/sim/lib/table/__tests__/update-row.test.ts +++ b/apps/sim/lib/table/__tests__/update-row.test.ts @@ -26,12 +26,6 @@ vi.mock('@/lib/table/billing', () => ({ TableRowLimitError: class TableRowLimitError extends Error {}, })) -// These suites assert flag-off position-shift semantics; pin the flag so they're -// deterministic regardless of a local TABLES_FRACTIONAL_ORDERING env value. -vi.mock('@/lib/core/config/feature-flags', () => ({ - isFeatureEnabled: vi.fn().mockResolvedValue(false), -})) - vi.mock('@/lib/table/validation', () => ({ validateRowSize: vi.fn(() => ({ valid: true, errors: [] })), validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })), @@ -187,29 +181,17 @@ describe('insertRow — position race safety (migration 0198 + advisory lock)', expect(findExecutedSqlContaining('hashtextextended')).toBe(true) }) - it('explicit-position inserts also acquire the advisory lock to serialize position shifts', async () => { - dbChainMockFns.limit.mockResolvedValueOnce([]) - dbChainMockFns.returning.mockResolvedValueOnce([ - { - id: 'row-1', - tableId: 'tbl-1', - workspaceId: 'ws-1', - data: { name: 'a' }, - position: 5, - createdAt: new Date(), - updatedAt: new Date(), - }, - ]) - - await insertRow( - { tableId: 'tbl-1', data: { name: 'a' }, workspaceId: 'ws-1', position: 5 }, - TABLE, - 'req-1' - ) + it('explicit-position inserts also acquire the advisory lock to serialize order-key minting', async () => { + await expect( + insertRow( + { tableId: 'tbl-1', data: { name: 'a' }, workspaceId: 'ws-1', position: 5 }, + TABLE, + 'req-1' + ) + ).rejects.toBeDefined() - // `(table_id, position)` index is non-unique, so concurrent explicit-position - // inserts at the same slot could both skip the shift and duplicate — lock - // serializes them. + // A position-based insert resolves its order_key from the neighbor at that + // rank; the lock serializes concurrent minting at the same slot. expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true) }) @@ -225,42 +207,6 @@ describe('insertRow — position race safety (migration 0198 + advisory lock)', expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true) }) - it('batchInsertRows with explicit positions acquires the advisory lock', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([ - { - id: 'row-1', - tableId: 'tbl-1', - workspaceId: 'ws-1', - data: { name: 'a' }, - position: 3, - createdAt: new Date(), - updatedAt: new Date(), - }, - { - id: 'row-2', - tableId: 'tbl-1', - workspaceId: 'ws-1', - data: { name: 'b' }, - position: 4, - createdAt: new Date(), - updatedAt: new Date(), - }, - ]) - - await batchInsertRows( - { - tableId: 'tbl-1', - rows: [{ name: 'a' }, { name: 'b' }], - workspaceId: 'ws-1', - positions: [3, 4], - }, - TABLE, - 'req-1' - ) - - expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true) - }) - it('upsertRow skips the advisory lock on the update path (match found)', async () => { vi.mocked(getUniqueColumns).mockReturnValue([{ name: 'name', type: 'string', unique: true }]) dbChainMockFns.limit.mockResolvedValueOnce([ diff --git a/apps/sim/lib/table/rows/ordering.ts b/apps/sim/lib/table/rows/ordering.ts index 7646f8c0a1d..368c9959d9e 100644 --- a/apps/sim/lib/table/rows/ordering.ts +++ b/apps/sim/lib/table/rows/ordering.ts @@ -8,8 +8,7 @@ import { db } from '@sim/db' import { userTableRows } from '@sim/db/schema' -import { and, asc, desc, eq, gt, gte, inArray, lt, lte, type SQL, sql } from 'drizzle-orm' -import { isFeatureEnabled } from '@/lib/core/config/feature-flags' +import { and, asc, desc, eq, gt, inArray, lt, lte, type SQL, sql } from 'drizzle-orm' import type { DbOrTx } from '@/lib/db/types' import { TABLE_LIMITS } from '@/lib/table/constants' import { keyBetween, nKeysBetween } from '@/lib/table/order-key' @@ -75,116 +74,15 @@ export async function maxOrderKey(executor: DbOrTx, tableId: string): Promise { - await acquireRowOrderLock(trx, tableId) - if (requestedPosition === undefined) { - return nextRowPosition(trx, tableId) - } - const [existing] = await trx - .select({ id: userTableRows.id }) - .from(userTableRows) - .where(and(eq(userTableRows.tableId, tableId), eq(userTableRows.position, requestedPosition))) - .limit(1) - if (existing) { - await shiftRowsUpFrom(trx, tableId, requestedPosition) - } - return requestedPosition -} - -/** - * Reserves positions for a batch of `count` rows. Opens each requested slot - * (ascending, preserving prior gaps) and returns the requested positions in - * original order; otherwise returns a contiguous append range. - */ -export async function reserveBatchPositions( - trx: DbTransaction, - tableId: string, - count: number, - requestedPositions?: number[] -): Promise { - await acquireRowOrderLock(trx, tableId) - if (requestedPositions && requestedPositions.length > 0) { - for (const pos of [...requestedPositions].sort((a, b) => a - b)) { - await shiftRowsUpFrom(trx, tableId, pos) - } - return requestedPositions - } - const start = await nextRowPosition(trx, tableId) - return Array.from({ length: count }, (_, i) => start + i) -} - -/** - * Recompacts row positions to be contiguous after a bulk delete. With - * `minDeletedPos`, only rows at/after it are re-numbered; single-row deletes use - * the cheaper {@link shiftRowsDownAfter}. - */ -export async function compactPositions( - trx: DbTransaction, - tableId: string, - minDeletedPos?: number -) { - if (minDeletedPos === undefined) { - await trx.execute(sql` - UPDATE user_table_rows t - SET position = r.new_pos - FROM ( - SELECT id, ROW_NUMBER() OVER (ORDER BY position) - 1 AS new_pos - FROM user_table_rows - WHERE table_id = ${tableId} - ) r - WHERE t.id = r.id AND t.table_id = ${tableId} AND t.position != r.new_pos - `) - return - } - await trx.execute(sql` - UPDATE user_table_rows t - SET position = r.new_pos - FROM ( - SELECT id, ${minDeletedPos}::int + ROW_NUMBER() OVER (ORDER BY position) - 1 AS new_pos - FROM user_table_rows - WHERE table_id = ${tableId} AND position >= ${minDeletedPos} - ) r - WHERE t.id = r.id AND t.table_id = ${tableId} AND t.position != r.new_pos - `) -} - /** * Computes the fractional `order_key` for a row inserted at the integer * `requestedPosition` (or appended when omitted). Used by position-based callers * (mothership tool, v1 API, undo position-fallback, transient old clients). * - * The neighbor at slot `s` is resolved differently per flag state: - * - **off**: `WHERE position = s` (positions are contiguous, so the row at - * position `s` is the `s`-th row — an indexed O(1) lookup). - * - **on**: the `s`-th row in `order_key, id` order (`OFFSET s`) — positions are - * gappy and non-authoritative, so `position = s` would miss; the visual - * ordinal is the key's ordinal. O(s), acceptable for these low-volume callers. + * The neighbor at slot `s` is the `s`-th row in `order_key, id` order (`OFFSET + * s`) — positions are gappy and non-authoritative, so `position = s` would miss; + * the visual ordinal is the key's ordinal. O(s), acceptable for these low-volume + * callers. * * Caller holds the row-order lock. */ @@ -193,24 +91,15 @@ export async function resolveInsertOrderKey( tableId: string, requestedPosition?: number ): Promise { - const fractionalOrdering = await isFeatureEnabled('tables-fractional-ordering') const orderKeyAtSlot = async (slot: number): Promise => { if (slot < 0) return null - if (fractionalOrdering) { - const [r] = await trx - .select({ orderKey: userTableRows.orderKey }) - .from(userTableRows) - .where(eq(userTableRows.tableId, tableId)) - .orderBy(asc(userTableRows.orderKey), asc(userTableRows.id)) - .limit(1) - .offset(slot) - return r?.orderKey ?? null - } const [r] = await trx .select({ orderKey: userTableRows.orderKey }) .from(userTableRows) - .where(and(eq(userTableRows.tableId, tableId), eq(userTableRows.position, slot))) + .where(eq(userTableRows.tableId, tableId)) + .orderBy(asc(userTableRows.orderKey), asc(userTableRows.id)) .limit(1) + .offset(slot) return r?.orderKey ?? null } if (requestedPosition === undefined) { @@ -225,19 +114,17 @@ export async function resolveInsertOrderKey( * Resolves the `order_key` for an insert expressed by an anchor row id — * `afterRowId` (place directly after) or `beforeRowId` (directly before). Finds * the anchor and its adjacent key via the `(table_id, order_key, id)` index - * (O(1)) and mints a key between them. Also returns a legacy integer `position` - * (anchor's position ±) so the flag-off shift path still works. Caller holds the - * row-order lock. + * (O(1)) and mints a key between them. Caller holds the row-order lock. */ export async function resolveInsertByNeighbor( trx: DbTransaction, tableId: string, afterRowId?: string, beforeRowId?: string -): Promise<{ orderKey: string; position: number }> { +): Promise { const anchorId = afterRowId ?? beforeRowId! const [anchor] = await trx - .select({ orderKey: userTableRows.orderKey, position: userTableRows.position }) + .select({ orderKey: userTableRows.orderKey }) .from(userTableRows) .where(and(eq(userTableRows.tableId, tableId), eq(userTableRows.id, anchorId))) .limit(1) @@ -245,12 +132,10 @@ export async function resolveInsertByNeighbor( // stale view) is an error, not a silent insert at the front. if (!anchor) throw new Error(`Row not found: ${anchorId}`) const anchorKey = anchor.orderKey ?? null - // A null key on the anchor means the table isn't backfilled. With the flag on - // (key is authoritative) the adjacent-key lookup below can't work — fail - // loudly rather than mint a wrong key. Flag off keeps `position` authoritative, - // so a best-effort key here is fine (the backfill re-keys before the flip). - const fractionalOrdering = await isFeatureEnabled('tables-fractional-ordering') - if (anchorKey === null && fractionalOrdering) { + // A null key on the anchor means the table isn't backfilled. order_key is + // authoritative, so the adjacent-key lookup below can't work — fail loudly + // rather than mint a wrong key. + if (anchorKey === null) { throw new Error(`Row ${anchorId} has no order_key yet (table not backfilled)`) } @@ -259,82 +144,45 @@ export async function resolveInsertByNeighbor( // (not the `(order_key, id)` row tuple) skips past any sibling that shares the // anchor's key, so `keyBetween` always gets strictly-ordered bounds and can't // throw on a stray duplicate. Identical to the row tuple when keys are distinct. - // A null anchorKey (flag off, un-backfilled) has no key to compare — leave the - // upper bound open, matching the prior best-effort behavior. - let nextKey: string | null = null - if (anchorKey !== null) { - const [next] = await trx - .select({ orderKey: userTableRows.orderKey }) - .from(userTableRows) - .where(and(eq(userTableRows.tableId, tableId), gt(userTableRows.orderKey, anchorKey))) - .orderBy(asc(userTableRows.orderKey)) - .limit(1) - nextKey = next?.orderKey ?? null - } - return { - orderKey: keyBetween(anchorKey, nextKey), - position: anchor.position + 1, - } - } - - // beforeRowId: lo = the largest key strictly LESS than the anchor key (distinct, - // same rationale as the afterRowId branch above). - let prevKey: string | null = null - if (anchorKey !== null) { - const [prev] = await trx + const [next] = await trx .select({ orderKey: userTableRows.orderKey }) .from(userTableRows) - .where(and(eq(userTableRows.tableId, tableId), lt(userTableRows.orderKey, anchorKey))) - .orderBy(desc(userTableRows.orderKey)) + .where(and(eq(userTableRows.tableId, tableId), gt(userTableRows.orderKey, anchorKey))) + .orderBy(asc(userTableRows.orderKey)) .limit(1) - prevKey = prev?.orderKey ?? null - } - return { - orderKey: keyBetween(prevKey, anchorKey), - position: anchor.position, + return keyBetween(anchorKey, next?.orderKey ?? null) } + + // beforeRowId: lo = the largest key strictly LESS than the anchor key (distinct, + // same rationale as the afterRowId branch above). + const [prev] = await trx + .select({ orderKey: userTableRows.orderKey }) + .from(userTableRows) + .where(and(eq(userTableRows.tableId, tableId), lt(userTableRows.orderKey, anchorKey))) + .orderBy(desc(userTableRows.orderKey)) + .limit(1) + return keyBetween(prev?.orderKey ?? null, anchorKey) } /** - * Computes fractional `order_key`s for a batch insert. With no `positions`, - * appends a contiguous run after the current max key. With explicit `positions` - * (undo restore), keys each row between its pre-shift position neighbors — - * correct because requested positions are distinct. Caller holds the lock. - * - * The explicit-`positions` path is meaningful only when `position` is - * authoritative (flag off): with the flag on, a saved `position` is a gappy - * column value, not a visual rank, so feeding it to {@link resolveInsertOrderKey} - * (which reads `position` as an `OFFSET` rank under the flag) would mint keys at - * the wrong ranks. Callers needing exact placement under the flag pass - * `orderKeys` (handled before this function); here we just append a run. + * Computes fractional `order_key`s for a batch insert by appending a contiguous + * run after the current max key. `order_key` is authoritative, so callers needing + * exact placement pass explicit `orderKeys` (handled before this function); here + * we just append a run. Caller holds the lock. */ export async function resolveBatchInsertOrderKeys( trx: DbTransaction, tableId: string, - count: number, - positions?: number[] + count: number ): Promise { - if ( - !positions || - positions.length === 0 || - (await isFeatureEnabled('tables-fractional-ordering')) - ) { - return nKeysBetween(await maxOrderKey(trx, tableId), null, count) - } - const keys: string[] = [] - for (const pos of positions) { - keys.push(await resolveInsertOrderKey(trx, tableId, pos)) - } - return keys + return nKeysBetween(await maxOrderKey(trx, tableId), null, count) } /** - * Inserts a single row in its own transaction. Always assigns a fractional - * `order_key`. When the fractional-ordering flag is on, `order_key` is - * authoritative and `position` is a best-effort append (no O(N) shift); when - * off, `position` is reserved as before (shifting to open the slot). Validation - * and side-effect dispatch stay with the caller; capacity is enforced by the - * `increment_user_table_row_count` trigger. + * Inserts a single row in its own transaction. Assigns a fractional `order_key` + * (authoritative) and a best-effort append `position` (no O(N) shift). + * Validation and side-effect dispatch stay with the caller; capacity is enforced + * by the `increment_user_table_row_count` trigger. */ export async function insertOrderedRow(params: { tableId: string @@ -360,35 +208,15 @@ export async function insertOrderedRow(params: { await setTableTxTimeouts(trx) await acquireRowOrderLock(trx, tableId) - const fractionalOrdering = await isFeatureEnabled('tables-fractional-ordering') + // Resolve the authoritative order key from neighbor ids when given, else from + // the requested position. + const orderKey = + afterRowId || beforeRowId + ? await resolveInsertByNeighbor(trx, tableId, afterRowId, beforeRowId) + : await resolveInsertOrderKey(trx, tableId, position) - // Resolve the order key (and a legacy slot position for the flag-off shift - // path) from neighbor ids when given, else from the requested position. - let orderKey: string - let slotPosition = position - if (afterRowId || beforeRowId) { - const resolved = await resolveInsertByNeighbor(trx, tableId, afterRowId, beforeRowId) - orderKey = resolved.orderKey - slotPosition = resolved.position - } else { - orderKey = await resolveInsertOrderKey(trx, tableId, position) - } - - let targetPosition: number - if (fractionalOrdering) { - // order_key is authoritative — keep a best-effort, no-shift position. - targetPosition = await nextRowPosition(trx, tableId) - } else if (slotPosition !== undefined) { - const [existing] = await trx - .select({ id: userTableRows.id }) - .from(userTableRows) - .where(and(eq(userTableRows.tableId, tableId), eq(userTableRows.position, slotPosition))) - .limit(1) - if (existing) await shiftRowsUpFrom(trx, tableId, slotPosition) - targetPosition = slotPosition - } else { - targetPosition = await nextRowPosition(trx, tableId) - } + // order_key is authoritative — keep a best-effort, no-shift position. + const targetPosition = await nextRowPosition(trx, tableId) return trx .insert(userTableRows) @@ -416,8 +244,9 @@ export async function insertOrderedRow(params: { } /** - * Deletes a single row by id in its own transaction, then closes the positional - * gap. Returns `false` when no row matched. + * Deletes a single row by id in its own transaction. Deleting a row never changes + * another row's `order_key`, so no positional reshift is needed. Returns `false` + * when no row matched. */ export async function deleteOrderedRow(params: { tableId: string @@ -436,33 +265,27 @@ export async function deleteOrderedRow(params: { eq(userTableRows.workspaceId, workspaceId) ) ) - .returning({ position: userTableRows.position }) - if (!deleted) return false - // Fractional ordering: deleting a row never changes another row's order_key, - // so the O(N) position reshift is skipped entirely. - if (!(await isFeatureEnabled('tables-fractional-ordering'))) { - await shiftRowsDownAfter(trx, tableId, deleted.position) - } - return true + .returning({ id: userTableRows.id }) + return Boolean(deleted) }) } /** - * Deletes the given row ids in batches within one transaction, then recompacts - * positions from the earliest deleted slot. Returns the deleted rows (id + prior - * position). The caller resolves which ids to delete (used by both delete-by-ids - * and delete-by-filter). + * Deletes the given row ids in batches within one transaction. Deletes leave + * `order_key` untouched, so no positional recompaction is needed. Returns the + * deleted row ids. The caller resolves which ids to delete (used by both + * delete-by-ids and delete-by-filter). */ export async function deleteOrderedRowsByIds(params: { tableId: string workspaceId: string rowIds: string[] -}): Promise<{ id: string; position: number }[]> { +}): Promise<{ id: string }[]> { const { tableId, workspaceId, rowIds } = params if (rowIds.length === 0) return [] return db.transaction(async (trx) => { await setTableTxTimeouts(trx, { statementMs: 60_000 }) - const deleted: { id: string; position: number }[] = [] + const deleted: { id: string }[] = [] for (let i = 0; i < rowIds.length; i += TABLE_LIMITS.DELETE_BATCH_SIZE) { const batch = rowIds.slice(i, i + TABLE_LIMITS.DELETE_BATCH_SIZE) const rows = await trx @@ -474,17 +297,9 @@ export async function deleteOrderedRowsByIds(params: { inArray(userTableRows.id, batch) ) ) - .returning({ id: userTableRows.id, position: userTableRows.position }) + .returning({ id: userTableRows.id }) deleted.push(...rows) } - // Fractional ordering: deletes leave order_key untouched, so no recompaction. - if (!(await isFeatureEnabled('tables-fractional-ordering')) && deleted.length > 0) { - const minDeletedPos = deleted.reduce( - (min, r) => (r.position < min ? r.position : min), - deleted[0].position - ) - await compactPositions(trx, tableId, minDeletedPos) - } return deleted }) } diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 75497ad8aeb..fe12af429b3 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -16,7 +16,6 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, count, eq, inArray, lte, notInArray, type SQL, sql } from 'drizzle-orm' -import { isFeatureEnabled } from '@/lib/core/config/feature-flags' import { assertRowCapacity, getMaxRowsPerTable, @@ -41,8 +40,6 @@ import { deleteOrderedRowsByIds, insertOrderedRow, nextRowPosition, - reserveBatchPositions, - reserveInsertPosition, resolveBatchInsertOrderKeys, resolveInsertOrderKey, } from '@/lib/table/rows/ordering' @@ -281,20 +278,14 @@ export async function batchInsertRowsWithTx( }) await acquireRowOrderLock(trx, data.tableId) - const fractionalOrdering = await isFeatureEnabled('tables-fractional-ordering') - // Undo restore passes exact saved keys; otherwise derive from positions/append. + // Undo restore passes exact saved keys; otherwise append after the current max. const orderKeys = data.orderKeys && data.orderKeys.length > 0 ? data.orderKeys - : await resolveBatchInsertOrderKeys(trx, data.tableId, data.rows.length, data.positions) - let positions: number[] - if (fractionalOrdering) { - // order_key authoritative — best-effort append positions, no shift. - const start = await nextRowPosition(trx, data.tableId) - positions = Array.from({ length: data.rows.length }, (_, i) => start + i) - } else { - positions = await reserveBatchPositions(trx, data.tableId, data.rows.length, data.positions) - } + : await resolveBatchInsertOrderKeys(trx, data.tableId, data.rows.length) + // order_key is authoritative — best-effort append positions, no shift. + const start = await nextRowPosition(trx, data.tableId) + const positions = Array.from({ length: data.rows.length }, (_, i) => start + i) const rowsToInsert = data.rows.map((rowData, i) => buildRow(rowData, positions[i], orderKeys[i])) const insertedRows = await trx.insert(userTableRows).values(rowsToInsert).returning() @@ -668,7 +659,7 @@ export async function upsertRow( tableId: data.tableId, workspaceId: data.workspaceId, data: data.data, - position: await reserveInsertPosition(trx, data.tableId), + position: await nextRowPosition(trx, data.tableId), orderKey: await resolveInsertOrderKey(trx, data.tableId), createdAt: now, updatedAt: now, @@ -738,18 +729,17 @@ export async function upsertRow( /** * Canonical ORDER BY for a table's rows, shared by `queryRows` (the paginated * list) and `findRowMatches` so a match's ordinal lines up with its index in - * the list. Order: explicit data sort (if any) → fractional `order_key` or - * legacy `position` → `id`. The `id` tiebreak is always appended so equal - * positions order deterministically — without it two separate query executions - * (a find vs a list page) could shuffle ties and misalign ordinals. + * the list. Order: explicit data sort (if any) → fractional `order_key` → `id`. + * The `id` tiebreak is always appended so equal keys order deterministically — + * without it two separate query executions (a find vs a list page) could shuffle + * ties and misalign ordinals. */ function buildRowOrderBySql( sort: Sort | undefined, tableName: string, - columns: ColumnDefinition[], - fractionalOrderingEnabled: boolean + columns: ColumnDefinition[] ): SQL { - const primary = fractionalOrderingEnabled ? `${tableName}.order_key` : `${tableName}.position` + const primary = `${tableName}.order_key` const id = `${tableName}.id` if (sort && Object.keys(sort).length > 0) { const sortClause = buildSortClause(sort, tableName, columns) @@ -814,8 +804,7 @@ export async function findRowMatches( if (filterClause) whereClause = and(baseConditions, filterClause) } - const fractionalOrdering = await isFeatureEnabled('tables-fractional-ordering') - const orderBySql = buildRowOrderBySql(options.sort, tableName, columns, fractionalOrdering) + const orderBySql = buildRowOrderBySql(options.sort, tableName, columns) const pattern = `%${escapeLikePattern(options.q)}%` const result = await db.transaction(async (trx) => { @@ -969,10 +958,7 @@ export async function queryRows( // Hide rows a running delete job is about to remove — both the page and the count below share // this clause, so totals stay consistent with the visible rows. - const [deleteMask, fractionalOrdering] = await Promise.all([ - pendingDeleteMask(table), - isFeatureEnabled('tables-fractional-ordering'), - ]) + const deleteMask = await pendingDeleteMask(table) const baseConditions = and( eq(userTableRows.tableId, table.id), @@ -1005,7 +991,7 @@ export async function queryRows( .select() .from(userTableRows) .where(pageWhere ?? baseConditions) - .orderBy(buildRowOrderBySql(sort, tableName, columns, fractionalOrdering)) + .orderBy(buildRowOrderBySql(sort, tableName, columns)) return after ? query.limit(limit) : query.limit(limit).offset(offset) } diff --git a/apps/sim/lib/table/types.ts b/apps/sim/lib/table/types.ts index ba08af9c83e..26dcb886bba 100644 --- a/apps/sim/lib/table/types.ts +++ b/apps/sim/lib/table/types.ts @@ -372,8 +372,8 @@ export interface TableRow { executions: RowExecutions position: number /** - * Fractional order key. Authoritative row order when `TABLES_FRACTIONAL_ORDERING` - * is on; absent only for rows not yet backfilled (clients fall back to `position`). + * Fractional order key — the authoritative row order. Absent only for rows not + * yet backfilled (clients fall back to `position`). */ orderKey?: string createdAt: Date | string @@ -526,11 +526,9 @@ export interface BatchInsertData { rows: RowData[] workspaceId: string userId?: string - /** Optional per-row target positions. Length must equal `rows.length`. */ - positions?: number[] /** * Optional per-row exact order keys (undo restore re-inserts at the saved key). - * Length must equal `rows.length`. Takes precedence over `positions`. + * Length must equal `rows.length`. */ orderKeys?: string[] } diff --git a/apps/sim/stores/table/types.ts b/apps/sim/stores/table/types.ts index 17e72fb8c0a..55521402cef 100644 --- a/apps/sim/stores/table/types.ts +++ b/apps/sim/stores/table/types.ts @@ -32,7 +32,6 @@ export type TableUndoAction = | { type: 'create-row' rowId: string - position: number orderKey?: string data?: Record } @@ -40,7 +39,6 @@ export type TableUndoAction = type: 'create-rows' rows: Array<{ rowId: string - position: number orderKey?: string data: Record }> diff --git a/packages/db/schema.ts b/packages/db/schema.ts index b7d03c020df..c46ffa265ec 100644 --- a/packages/db/schema.ts +++ b/packages/db/schema.ts @@ -3389,9 +3389,9 @@ export const userTableRows = pgTable( data: jsonb('data').notNull(), position: integer('position').notNull().default(0), /** - * Fractional order key (base-62 string). Authoritative row order when the - * `TABLES_FRACTIONAL_ORDERING` flag is on; nullable during the backfill - * window. Ordered with `id` as a deterministic tiebreaker. + * Fractional order key (base-62 string) — the authoritative row order. + * Nullable during the backfill window. Ordered with `id` as a deterministic + * tiebreaker. * * Stored with `COLLATE "C"` (migration 0228) so Postgres compares it bytewise, * matching the fractional-indexing library's ASCII ordering. drizzle can't diff --git a/packages/db/script-migrations/0001_backfill_table_order_keys.ts b/packages/db/script-migrations/0001_backfill_table_order_keys.ts index ea0e368f0ce..371d8e32edc 100644 --- a/packages/db/script-migrations/0001_backfill_table_order_keys.ts +++ b/packages/db/script-migrations/0001_backfill_table_order_keys.ts @@ -16,7 +16,7 @@ const WRITE_CHUNK_SIZE = 5000 * Row ordering moved from the contiguous integer `position` to a fractional * string `order_key` (O(1) insert/delete — no reshift/recompact). Each existing * row gets a key derived from its current `position` order, so the fractional - * ordering matches today's once `TABLES_FRACTIONAL_ORDERING` is on. + * ordering matches the legacy ordering for rows that predate the column. * * Per-table-atomic: each table is keyed inside one transaction holding the same * per-table advisory lock the app uses for inserts, so a concurrent insert diff --git a/packages/testing/src/mocks/database.mock.ts b/packages/testing/src/mocks/database.mock.ts index 17a039b5d63..9a879e4cad8 100644 --- a/packages/testing/src/mocks/database.mock.ts +++ b/packages/testing/src/mocks/database.mock.ts @@ -100,7 +100,15 @@ export function createMockSqlOperators() { * }) * ``` */ -const limit = vi.fn(() => Promise.resolve([] as unknown[])) +const offset = vi.fn(() => Promise.resolve([] as unknown[])) +// `.limit()` returns a builder that is awaitable (default empty page) and also +// exposes `.offset()` for keyset/OFFSET paging (`.limit(n).offset(m)`). +const limitBuilder = () => { + const thenable: any = Promise.resolve([] as unknown[]) + thenable.offset = offset + return thenable +} +const limit = vi.fn(limitBuilder) const returning = vi.fn(() => Promise.resolve([] as unknown[])) const execute = vi.fn(() => Promise.resolve([] as unknown[])) @@ -169,6 +177,7 @@ export const dbChainMockFns = { from, where, limit, + offset, orderBy, returning, innerJoin, @@ -211,7 +220,8 @@ export function resetDbChainMock(): void { update.mockImplementation(() => ({ set })) set.mockImplementation(() => ({ where })) del.mockImplementation(() => ({ where })) - limit.mockImplementation(() => Promise.resolve([] as unknown[])) + limit.mockImplementation(limitBuilder) + offset.mockImplementation(() => Promise.resolve([] as unknown[])) orderBy.mockImplementation(terminalBuilder) returning.mockImplementation(() => Promise.resolve([] as unknown[])) having.mockImplementation(terminalBuilder) diff --git a/packages/testing/src/mocks/env-flags.mock.ts b/packages/testing/src/mocks/env-flags.mock.ts index 480ae0d62f0..590a5b9fde6 100644 --- a/packages/testing/src/mocks/env-flags.mock.ts +++ b/packages/testing/src/mocks/env-flags.mock.ts @@ -21,7 +21,6 @@ export const envFlagsMock = { isRegistrationDisabled: false, isEmailPasswordEnabled: false, isTriggerDevEnabled: false, - isTablesFractionalOrderingEnabled: false, isSsoEnabled: false, isAccessControlEnabled: false, isOrganizationsEnabled: false,