Skip to content

Commit 2fd8743

Browse files
committed
improvement(tables): select ids only for capped deletes and gate only trigger-dependent tests
1 parent ba7975d commit 2fd8743

2 files changed

Lines changed: 75 additions & 26 deletions

File tree

‎apps/sim/lib/table/rows/row-writes.integration.ts‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/**
22
* Row-write integration tests against the provisioned, disposable TEST_DATABASE_URL database (the
3-
* integration setup points DATABASE_URL at it too). The migrated `rows_version` triggers, the
4-
* row-order advisory lock, and the filtered bulk-mutation queries all run for real.
3+
* integration setup points DATABASE_URL at it too). The row-order advisory lock and the filtered
4+
* bulk-mutation queries run for real everywhere; the `rows_version` cases also need the migrated
5+
* triggers and skip on a `db:push` schema.
56
*/
67
import { db } from '@sim/db'
78
import { userTableDefinitions, userTableRows } from '@sim/db/schema'
@@ -44,11 +45,15 @@ const control = postgres(url, { max: 4, onnotice: () => {} })
4445
const workspaceId = generateId()
4546
const userId = generateId()
4647

47-
/** `db:push` never installs the migration-only triggers these assertions depend on. */
48-
const [{ migrated }] = await control<{ migrated: boolean }[]>`SELECT EXISTS (
49-
SELECT 1 FROM pg_trigger WHERE tgname = 'user_table_rows_version_update_trigger'
50-
) AS migrated`
51-
if (!migrated) await control.end()
48+
/**
49+
* `db:push` never installs migration-only objects: the `rows_version` triggers and the
50+
* `(workspace_id, table_id)` dependency statistic. Only the cases that depend on them need them.
51+
*/
52+
const [{ migrated }] = await control<{ migrated: boolean }[]>`SELECT
53+
EXISTS (SELECT 1 FROM pg_trigger WHERE tgname = 'user_table_rows_version_update_trigger')
54+
AND EXISTS (
55+
SELECT 1 FROM pg_statistic_ext WHERE stxname = 'user_table_rows_workspace_table_stats'
56+
) AS migrated`
5257

5358
async function createTable(columns: ColumnDefinition[]): Promise<TableDefinition> {
5459
const id = generateId()
@@ -85,7 +90,7 @@ function planNodes(plan: QueryPlan): QueryPlan[] {
8590
const textColumns = (...ids: string[]): ColumnDefinition[] =>
8691
ids.map((id) => ({ id, name: id, type: 'string' }))
8792

88-
describe.skipIf(!migrated)('table row writes against real PostgreSQL', () => {
93+
describe('table row writes against real PostgreSQL', () => {
8994
beforeAll(async () => {
9095
await control`INSERT INTO "user" (id, name, email, email_verified, created_at, updated_at)
9196
VALUES (${userId}, 'Row write fixture', ${`${userId}@example.test`}, true, now(), now())`
@@ -103,7 +108,7 @@ describe.skipIf(!migrated)('table row writes against real PostgreSQL', () => {
103108
await control.end()
104109
})
105110

106-
describe('rows_version', () => {
111+
describe.skipIf(!migrated)('rows_version', () => {
107112
it('advances once for a transaction that edits cells across several statements', async () => {
108113
const table = await createTable(textColumns('name'))
109114
await seedRows(table.id, [
@@ -379,7 +384,8 @@ describe.skipIf(!migrated)('table row writes against real PostgreSQL', () => {
379384
expect([...result.affectedRowIds].sort()).toEqual([...expected].sort())
380385
})
381386

382-
it('fences only a containment filter into a GIN probe and leaves any other filter unfenced', async () => {
387+
/** A table large enough that the planner's index choice reflects its statistics. */
388+
async function seedPlanFixture(): Promise<(filter: Filter) => Promise<QueryPlan[]>> {
383389
const table = await createTable(textColumns('kind', 'n'))
384390
await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data, order_key)
385391
SELECT ${table.id} || '-' || n, ${table.id}, ${workspaceId},
@@ -388,7 +394,7 @@ describe.skipIf(!migrated)('table row writes against real PostgreSQL', () => {
388394
FROM generate_series(1, 20000) AS n`
389395
await control`ANALYZE user_table_rows`
390396

391-
async function planFor(filter: Filter): Promise<QueryPlan[]> {
397+
return async (filter) => {
392398
const filterClause = buildFilterClause(
393399
filter,
394400
USER_TABLE_ROWS_SQL_NAME,
@@ -397,25 +403,35 @@ describe.skipIf(!migrated)('table row writes against real PostgreSQL', () => {
397403
if (!filterClause) throw new Error('Fixture filter compiled to no clause')
398404
const plans = await withSeqscanOff((trx) =>
399405
trx.execute<{ 'QUERY PLAN': { Plan: QueryPlan }[] }>(
400-
sql`EXPLAIN (FORMAT JSON) ${firstMatchingRowsQuery(table, filter, filterClause, 5)}`
406+
sql`EXPLAIN (FORMAT JSON) ${firstMatchingRowsQuery(table, filter, filterClause, 5, 'rows')}`
401407
)
402408
)
403409
return planNodes(plans[0]['QUERY PLAN'][0].Plan)
404410
}
411+
}
412+
413+
it('fences only a containment filter and leaves any other filter unfenced', async () => {
414+
const planFor = await seedPlanFixture()
405415

406416
const fenced = await planFor({ kind: 'hit' })
407417
expect(fenced.some((node) => node['Node Type'] === 'CTE Scan')).toBe(true)
418+
419+
const walked = await planFor({ kind: { $ne: 'miss' } })
420+
expect(walked.some((node) => node['Node Type'] === 'CTE Scan')).toBe(false)
421+
expect(walked[0]['Node Type']).toBe('Limit')
422+
})
423+
424+
it.skipIf(!migrated)('probes the tenant GIN index inside the fence', async () => {
425+
const planFor = await seedPlanFixture()
426+
427+
const fenced = await planFor({ kind: 'hit' })
408428
expect(
409429
fenced.some(
410430
(node) =>
411431
node['Node Type'] === 'Bitmap Index Scan' &&
412432
node['Index Name'] === 'user_table_rows_tenant_data_gin_idx'
413433
)
414434
).toBe(true)
415-
416-
const walked = await planFor({ kind: { $ne: 'miss' } })
417-
expect(walked.some((node) => node['Node Type'] === 'CTE Scan')).toBe(false)
418-
expect(walked[0]['Node Type']).toBe('Limit')
419435
})
420436
})
421437

‎apps/sim/lib/table/rows/service.ts‎

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -982,7 +982,8 @@ function buildRowOrderBySql(
982982
}
983983

984984
/**
985-
* The first `limit` rows matching `filter` in the default `(order_key, id)` order.
985+
* The first `limit` rows matching `filter` in the default `(order_key, id)` order, projected to
986+
* their ids alone or to `id` and `data`.
986987
*
987988
* As one ordered query, the planner walks the `(table_id, order_key, id)` index and tests each row
988989
* against the filter until `limit` match: cheap when matches are dense, the whole table when they
@@ -995,14 +996,19 @@ export function firstMatchingRowsQuery(
995996
table: TableDefinition,
996997
filter: Filter,
997998
filterClause: SQL,
998-
limit: number
999+
limit: number,
1000+
projection: 'ids' | 'rows'
9991001
): SQL {
1002+
const columns =
1003+
projection === 'rows'
1004+
? sql`${userTableRows.id}, ${userTableRows.data}`
1005+
: sql`${userTableRows.id}`
10001006
const tenantMatch = sql`${userTableRows.tableId} = ${table.id}
10011007
AND ${userTableRows.workspaceId} = ${table.workspaceId}
10021008
AND ${filterClause}`
10031009
if (!isContainmentOnlyFilter(filter, table.schema.columns)) {
10041010
return sql`
1005-
SELECT ${userTableRows.id}, ${userTableRows.data}
1011+
SELECT ${columns}
10061012
FROM ${userTableRows}
10071013
WHERE ${tenantMatch}
10081014
ORDER BY ${userTableRows.orderKey}, ${userTableRows.id}
@@ -1018,7 +1024,7 @@ export function firstMatchingRowsQuery(
10181024
kept AS (
10191025
SELECT id, order_key FROM matched ORDER BY order_key, id LIMIT ${limit}
10201026
)
1021-
SELECT ${userTableRows.id}, ${userTableRows.data}
1027+
SELECT ${columns}
10221028
FROM kept
10231029
INNER JOIN ${userTableRows} ON ${userTableRows.id} = kept.id
10241030
ORDER BY kept.order_key, kept.id
@@ -1029,11 +1035,26 @@ async function selectFirstMatchingRows(
10291035
table: TableDefinition,
10301036
filter: Filter,
10311037
filterClause: SQL,
1032-
limit: number
1033-
): Promise<Array<{ id: string; data: RowData }>> {
1038+
limit: number,
1039+
projection: 'ids'
1040+
): Promise<Array<{ id: string }>>
1041+
async function selectFirstMatchingRows(
1042+
table: TableDefinition,
1043+
filter: Filter,
1044+
filterClause: SQL,
1045+
limit: number,
1046+
projection: 'rows'
1047+
): Promise<Array<{ id: string; data: RowData }>>
1048+
async function selectFirstMatchingRows(
1049+
table: TableDefinition,
1050+
filter: Filter,
1051+
filterClause: SQL,
1052+
limit: number,
1053+
projection: 'ids' | 'rows'
1054+
): Promise<Array<{ id: string; data?: RowData }>> {
10341055
const rows = await withSeqscanOff(async (trx) =>
1035-
trx.execute<{ id: string; data: RowData }>(
1036-
firstMatchingRowsQuery(table, filter, filterClause, limit)
1056+
trx.execute<{ id: string; data?: RowData }>(
1057+
firstMatchingRowsQuery(table, filter, filterClause, limit, projection)
10371058
)
10381059
)
10391060
return Array.from(rows)
@@ -2370,7 +2391,13 @@ export async function updateRowsByFilter(
23702391
return { affectedCount: affectedRowIds.length, affectedRowIds }
23712392
}
23722393

2373-
const matchingRows = await selectFirstMatchingRows(table, data.filter, filterClause, limit)
2394+
const matchingRows = await selectFirstMatchingRows(
2395+
table,
2396+
data.filter,
2397+
filterClause,
2398+
limit,
2399+
'rows'
2400+
)
23742401
if (matchingRows.length === 0) {
23752402
return { affectedCount: 0, affectedRowIds: [] }
23762403
}
@@ -2745,7 +2772,13 @@ export async function deleteRowsByFilter(
27452772
if (page.length < TABLE_LIMITS.DELETE_PAGE_SIZE) break
27462773
}
27472774
} else {
2748-
const matchingRows = await selectFirstMatchingRows(table, data.filter, filterClause, limit)
2775+
const matchingRows = await selectFirstMatchingRows(
2776+
table,
2777+
data.filter,
2778+
filterClause,
2779+
limit,
2780+
'ids'
2781+
)
27492782
const rowIds = matchingRows.map((row) => row.id)
27502783
if (rowIds.length > 0) {
27512784
const deletedIds = await deleteOrderedRowsByIds({

0 commit comments

Comments
 (0)