diff --git a/apps/studio/data/table-rows/table-row-delete-mutation.test.ts b/apps/studio/data/table-rows/table-row-delete-mutation.test.ts new file mode 100644 index 00000000000..480f8bfdac5 --- /dev/null +++ b/apps/studio/data/table-rows/table-row-delete-mutation.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' + +import { getTableRowDeleteSql } from './table-row-delete-mutation' +import type { Entity } from '@/data/table-editor/table-editor-types' + +function createTable(primaryKeys: string[]): Entity { + return { + id: 1, + name: 'user_roles', + schema: 'public', + entity_type: 'r', + primary_keys: primaryKeys.map((name) => ({ name, schema: 'public', table_name: 'user_roles' })), + } as unknown as Entity +} + +describe('getTableRowDeleteSql', () => { + it('deletes by a single primary key with an in list', () => { + const sql = String( + getTableRowDeleteSql({ + table: createTable(['id']), + rows: [ + { idx: 0, id: 1 }, + { idx: 1, id: 2 }, + ], + }) + ) + + expect(sql).toBe('delete from public.user_roles where id in (1,2);') + }) + + it('deletes only the selected rows when the primary key is composite', () => { + const sql = String( + getTableRowDeleteSql({ + table: createTable(['user_id', 'role_id']), + rows: [ + { idx: 0, user_id: 1, role_id: 'admin' }, + { idx: 1, user_id: 2, role_id: 'viewer' }, + ], + }) + ) + + // Must match the row tuples, not each column independently. Matching + // columns independently would also delete (1, 'viewer') and (2, 'admin'). + expect(sql).toBe( + "delete from public.user_roles where (user_id, role_id) in ((1, 'admin'), (2, 'viewer'));" + ) + }) +}) diff --git a/apps/studio/data/table-rows/table-row-delete-mutation.tsx b/apps/studio/data/table-rows/table-row-delete-mutation.tsx index 9f5eb30b395..d4311bf83dc 100644 --- a/apps/studio/data/table-rows/table-row-delete-mutation.tsx +++ b/apps/studio/data/table-rows/table-row-delete-mutation.tsx @@ -29,13 +29,20 @@ export function getTableRowDeleteSql({ const { primaryKeys, error } = getPrimaryKeys({ table }) if (error) throw error - let queryChains = new Query().from(table.name, table.schema ?? undefined).delete() - primaryKeys?.forEach((key) => { - const primaryKeyValues = rows.map((x) => x[key]) - queryChains = queryChains.filter(key, 'in', primaryKeyValues) - }) + const queryChains = new Query().from(table.name, table.schema ?? undefined).delete() + if (!primaryKeys || primaryKeys.length === 0) return queryChains.toSql() - return queryChains.toSql() + if (primaryKeys.length === 1) { + const [key] = primaryKeys + const values = rows.map((row) => row[key]) + return queryChains.filter(key, 'in', values).toSql() + } + + // Composite primary keys must be matched as whole tuples. Filtering each key + // column independently matches the cartesian product of the selected values + // and would delete rows that were never selected. + const tuples = rows.map((row) => primaryKeys.map((key) => row[key])) + return queryChains.filter(primaryKeys, 'in', tuples).toSql() } export async function deleteTableRow({