diff --git a/.changeset/driver-sql-declared-indexes.md b/.changeset/driver-sql-declared-indexes.md new file mode 100644 index 000000000..76577ad73 --- /dev/null +++ b/.changeset/driver-sql-declared-indexes.md @@ -0,0 +1,26 @@ +--- +"@objectstack/driver-sql": minor +--- + +fix(driver-sql): materialize declared object-level indexes (#1459) + +The SQL driver synced columns and field-level `unique`, but silently dropped +object-level declared `indexes` (`ObjectSchema.indexes: [{ fields, unique }]`). +As a result several documented multi-column UNIQUE / dedup guarantees were +never enforced at the DB level — a fresh `dev --fresh` sqlite DB showed only +primary-key autoindexes. + +`initObjects` now materializes declared indexes (`syncDeclaredIndexes`) after +the table is created/altered: + +- single- and multi-column indexes, including `UNIQUE` +- NULL-distinct semantics (the cross-dialect default), so multiple NULL rows + stay insertable while non-NULL duplicates are rejected — matching the + convergence-on-conflict pattern the messaging pipeline relies on +- idempotent: deterministic, length-bounded index names + per-dialect + existing-index introspection (sqlite/pg/mysql); "already exists" races are + absorbed +- indexes referencing a non-materialized (virtual `formula`) column are skipped + with a warning instead of failing sync + +The `indexes` driver capability flag is now `true`. diff --git a/packages/plugins/driver-sql/src/sql-driver-schema.test.ts b/packages/plugins/driver-sql/src/sql-driver-schema.test.ts index 4ec012ed3..37e2c8a19 100644 --- a/packages/plugins/driver-sql/src/sql-driver-schema.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-schema.test.ts @@ -291,6 +291,148 @@ describe('SqlDriver Schema Sync (SQLite)', () => { expect(columns).toHaveProperty('display_name'); }); + it('should materialize a declared single-column unique index and reject duplicates', async () => { + const objects = [ + { + name: 'idx_unique_obj', + fields: { + slug: { type: 'string' }, + }, + indexes: [{ fields: ['slug'], unique: true }], + }, + ]; + + await driver.initObjects(objects as any); + + // PRAGMA proves a non-PK unique index was actually created. + const list: any = await knexInstance.raw('PRAGMA index_list(idx_unique_obj)'); + const declared = list.filter((i: any) => !String(i.name).startsWith('sqlite_autoindex')); + expect(declared.length).toBe(1); + expect(declared[0].unique).toBe(1); + + await driver.create('idx_unique_obj', { slug: 's1' }); + await expect(driver.create('idx_unique_obj', { slug: 's1' })).rejects.toThrow( + /UNIQUE constraint failed|duplicate key value/, + ); + }); + + it('should materialize a declared multi-column unique index (compound dedup)', async () => { + const objects = [ + { + name: 'idx_multi_obj', + fields: { + notification_id: { type: 'string' }, + recipient_id: { type: 'string' }, + channel: { type: 'string' }, + }, + indexes: [{ fields: ['notification_id', 'recipient_id', 'channel'], unique: true }], + }, + ]; + + await driver.initObjects(objects as any); + + const list: any = await knexInstance.raw('PRAGMA index_list(idx_multi_obj)'); + const declared = list.filter((i: any) => !String(i.name).startsWith('sqlite_autoindex')); + expect(declared.length).toBe(1); + expect(declared[0].unique).toBe(1); + + await driver.create('idx_multi_obj', { notification_id: 'n1', recipient_id: 'r1', channel: 'bell' }); + // Same tuple → rejected. + await expect( + driver.create('idx_multi_obj', { notification_id: 'n1', recipient_id: 'r1', channel: 'bell' }), + ).rejects.toThrow(/UNIQUE constraint failed|duplicate key value/); + // Differing channel → allowed. + await driver.create('idx_multi_obj', { notification_id: 'n1', recipient_id: 'r1', channel: 'email' }); + const rows = await driver.find('idx_multi_obj', {}); + expect(rows.length).toBe(2); + }); + + it('should allow multiple NULLs under a declared unique index (NULL-distinct)', async () => { + const objects = [ + { + name: 'idx_null_obj', + fields: { + dedup_key: { type: 'string' }, + }, + indexes: [{ fields: ['dedup_key'], unique: true }], + }, + ]; + + await driver.initObjects(objects as any); + + // Two rows with no dedup_key (NULL) must both be insertable. + await driver.create('idx_null_obj', {}); + await driver.create('idx_null_obj', {}); + const rows = await driver.find('idx_null_obj', {}); + expect(rows.length).toBe(2); + }); + + it('should materialize a declared non-unique index', async () => { + const objects = [ + { + name: 'idx_plain_obj', + fields: { + status: { type: 'string' }, + partition_key: { type: 'string' }, + }, + indexes: [{ fields: ['status', 'partition_key'] }], + }, + ]; + + await driver.initObjects(objects as any); + + const list: any = await knexInstance.raw('PRAGMA index_list(idx_plain_obj)'); + const declared = list.filter((i: any) => !String(i.name).startsWith('sqlite_autoindex')); + expect(declared.length).toBe(1); + expect(declared[0].unique).toBe(0); + }); + + it('should be idempotent when initObjects runs repeatedly with declared indexes', async () => { + const objects = [ + { + name: 'idx_idem_obj', + fields: { slug: { type: 'string' } }, + indexes: [{ fields: ['slug'], unique: true }], + }, + ]; + + await driver.initObjects(objects as any); + // Second run must not throw "index already exists". + await driver.initObjects(objects as any); + + const list: any = await knexInstance.raw('PRAGMA index_list(idx_idem_obj)'); + const declared = list.filter((i: any) => !String(i.name).startsWith('sqlite_autoindex')); + expect(declared.length).toBe(1); + }); + + it('should skip a declared index referencing a non-materialized (virtual) column', async () => { + const warnings: string[] = []; + (driver as any).logger = { warn: (msg: string) => warnings.push(msg) }; + + const objects = [ + { + name: 'idx_virtual_obj', + fields: { + name: { type: 'string' }, + total: { type: 'formula', expression: 'a + b', data_type: 'number' }, + }, + indexes: [{ fields: ['total'], unique: true }], + }, + ]; + + // Must not throw even though `total` has no physical column. + await driver.initObjects(objects as any); + + const list: any = await knexInstance.raw('PRAGMA index_list(idx_virtual_obj)'); + const declared = list.filter((i: any) => !String(i.name).startsWith('sqlite_autoindex')); + expect(declared.length).toBe(0); + expect(warnings.some((w) => w.includes('total'))).toBe(true); + }); + + it('reports indexes capability as supported', () => { + expect((driver as any).supports.indexes).toBe(true); + }); + it('should use short object name as physical table name in initObjects', async () => { const objects = [ { diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 5045e3b97..3d36ab5e4 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -13,6 +13,7 @@ import { StorageNameMapping } from '@objectstack/spec/system'; import { ExternalSchemaModeViolationError } from '@objectstack/spec/shared'; import knex, { Knex } from 'knex'; import { nanoid } from 'nanoid'; +import { createHash } from 'node:crypto'; /** * Default ID length for auto-generated IDs. @@ -134,7 +135,9 @@ export class SqlDriver implements IDataDriver { schemaSync: true, batchSchemaSync: false, migrations: false, - indexes: false, + // Object-level declared `indexes` (incl. multi-column UNIQUE) are + // materialized during `initObjects` — see `syncDeclaredIndexes`. + indexes: true, // Performance & Optimization connectionPooling: true, @@ -1128,6 +1131,128 @@ export class SqlDriver implements IDataDriver { } }); } + + // Materialize object-level declared indexes (`indexes: [{ fields, + // unique }]`). These are distinct from field-level `unique` (handled + // in `createColumn`) and carry the multi-column UNIQUE guarantees that + // dedup/convergence paths rely on (ADR-0030). Done after the table is + // created/altered so every referenced column physically exists. + const declaredIndexes = (obj as any).indexes; + if (Array.isArray(declaredIndexes) && declaredIndexes.length > 0) { + const colInfo = await this.knex(tableName).columnInfo(); + const physicalColumns = new Set(Object.keys(colInfo)); + await this.syncDeclaredIndexes(tableName, declaredIndexes, physicalColumns); + } + } + } + + /** + * Build a deterministic index name for a declared index so repeated + * `initObjects` runs converge on the same identifier (and can detect an + * already-materialized index by name). Long names are hash-suffixed to + * stay within the 63/64-char identifier limits of Postgres/MySQL. + */ + protected buildIndexName(tableName: string, fields: string[], unique: boolean): string { + const prefix = unique ? 'uniq' : 'idx'; + const base = `${prefix}_${tableName}_${fields.join('_')}`; + const MAX = 60; + if (base.length <= MAX) return base; + const hash = createHash('sha1').update(base).digest('hex').slice(0, 8); + return `${`${prefix}_${tableName}`.slice(0, MAX - 9)}_${hash}`; + } + + /** + * Read the names of indexes that already exist on a table, per dialect. + * Used to make declared-index sync idempotent across repeated runs. + * Failures are swallowed — at worst we attempt a create and absorb the + * "already exists" error in `syncDeclaredIndexes`. + */ + protected async getExistingIndexNames(tableName: string): Promise> { + const names = new Set(); + try { + if (this.isSqlite) { + const safe = tableName.replace(/[^a-zA-Z0-9_]/g, ''); + const rows: any = await this.knex.raw(`PRAGMA index_list(${safe})`); + for (const r of rows) names.add(r.name); + } else if (this.isPostgres) { + const res: any = await this.knex.raw( + `SELECT indexname FROM pg_indexes WHERE schemaname = 'public' AND tablename = ?`, + [tableName], + ); + for (const r of res.rows) names.add(r.indexname); + } else if (this.isMysql) { + const res: any = await this.knex.raw( + `SELECT INDEX_NAME FROM information_schema.STATISTICS WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ?`, + [tableName], + ); + for (const r of res[0]) names.add(r.INDEX_NAME); + } + } catch { + // Best-effort — fall through and let creation handle conflicts. + } + return names; + } + + /** + * Materialize declared object-level indexes. + * + * - Multi-column and single-column indexes are both supported. + * - `unique: true` emits a UNIQUE index. NULL-distinct semantics are the + * default across SQLite/Postgres/MySQL, so multiple NULL rows remain + * allowed while non-NULL duplicates are rejected — matching the + * convergence-on-conflict pattern the messaging pipeline relies on. + * - Idempotent: indexes already present (by deterministic name) are + * skipped, and an "already exists" race is absorbed. + * - Indexes referencing a column that wasn't materialized (e.g. a virtual + * `formula` field) are skipped with a warning rather than failing sync. + */ + protected async syncDeclaredIndexes( + tableName: string, + indexes: Array<{ name?: string; fields?: string[]; unique?: boolean }>, + physicalColumns: Set, + ): Promise { + const existing = await this.getExistingIndexNames(tableName); + + for (const idx of indexes) { + const fields = Array.isArray(idx?.fields) + ? idx.fields.filter((f): f is string => typeof f === 'string' && f.length > 0) + : []; + if (fields.length === 0) continue; + + const missing = fields.filter((f) => !physicalColumns.has(f)); + if (missing.length > 0) { + this.logger.warn( + `[sql-driver] skipping declared index on "${tableName}" — column(s) not materialized: ${missing.join(', ')}`, + { tableName, fields }, + ); + continue; + } + + const unique = idx.unique === true; + const name = + typeof idx.name === 'string' && idx.name.trim() + ? idx.name.trim() + : this.buildIndexName(tableName, fields, unique); + + if (existing.has(name)) continue; + + try { + await this.knex.schema.alterTable(tableName, (table) => { + if (unique) { + table.unique(fields, { indexName: name }); + } else { + table.index(fields, name); + } + }); + existing.add(name); + } catch (e: any) { + const msg = String(e?.message ?? e); + // A concurrent creator or a pre-existing equivalent index under a + // different name can race us here — both are benign for our intent + // (the index exists). Anything else is a real failure. + if (/already exists|duplicate key name|exists/i.test(msg)) continue; + throw e; + } } }