fix(driver-sql): materialize declared object-level indexes (#1459)

.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`.

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 = [
       {

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<Set<string>> {
+    const names = new Set<string>();
+    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<string>,
+  ): Promise<void> {
+    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;
+      }
     }
   }