fix(oracle): sequence keywords, create order, and tolerant index/trigger drops

apps/web/src/backend/modules/generated-ddl-live.test.ts

@@ -75,6 +75,14 @@ const TARGETS: Array<{
     provider: 'yugabytedb',
     options: { host: 'localhost', port: 5433, database: 'yugabyte', username: 'yugabyte', schema: 'public' },
   },
+  {
+    // Oracle has no bare `SELECT 1` either — it wants a FROM, and DUAL is it.
+    // Schema and user are the same thing here, so the "schema" is the account.
+    dialect: 'oracle',
+    provider: 'oracle',
+    options: { host: 'localhost', port: 1521, database: 'FOXDB', username: 'foxuser', password: 'foxpass', schema: 'FOXUSER' },
+    probe: 'SELECT 1 FROM DUAL',
+  },
   {
     // Slowest of the set to boot (the compose healthcheck allows two minutes
     // before it even starts probing) and the only one needing a native client
@@ -295,6 +303,24 @@ const ROUTINES: Record<
       `CREATE PROCEDURE dbo.touch_it_${TAG} AS BEGIN SELECT 1 END`,
     ],
   },
+  oracle: {
+    // An Oracle schema *is* a user, so the round trip needs a second account —
+    // which only a DBA can create, hence the admin credentials.
+    from: 'FOXUSER',
+    to: `FXRT${TAG.toUpperCase()}`,
+    admin: { username: 'system', password: 'FoxPass123' },
+    makeSchema: (s) =>
+      s === 'FOXUSER'
+        ? []
+        : [
+            `CREATE USER ${s} IDENTIFIED BY foxpass QUOTA UNLIMITED ON USERS`,
+            `GRANT CREATE SESSION, CREATE PROCEDURE TO ${s}`,
+          ],
+    ddl: (s) => [
+      `CREATE FUNCTION ${s}.DOUBLE_IT(X IN NUMBER) RETURN NUMBER AS BEGIN RETURN X * 2; END;`,
+      `CREATE PROCEDURE ${s}.TOUCH_IT AS BEGIN NULL; END;`,
+    ],
+  },
   db2: {
     from: `FXA${TAG}`,
     to: `FXB${TAG}`,

packages/sql/src/modules/sql-dialect.interface.ts

@@ -256,6 +256,15 @@ export interface SqlDialect {
    */
   wrapCreateSequence?(qualifiedName: string, createSql: string): string;
 
+  /**
+   * Oracle spells the negative sequence options as single keywords —
+   * `NOCYCLE`, `NOCACHE`. The spaced `NO CYCLE` / `NO CACHE` that Postgres and
+   * DB2 accept is **ORA-03049** there, which kills the CREATE SEQUENCE and then
+   * everything that depends on it: the table whose default calls the sequence,
+   * and the views over that table.
+   */
+  unspacedSequenceNoKeywords?: boolean;
+
   /**
    * Wrap the standard `ALTER SEQUENCE name ...;` into a dialect-safe form.
    * Called with the qualified name and the full `ALTER SEQUENCE name ...;` string.

packages/sql/src/modules/sql-generator.module.ts

@@ -349,6 +349,15 @@ export class SqlGeneratorModule {
    * index (SQL Server renders it as ALTER TABLE ADD CONSTRAINT). `idx.name` must be bare.
    */
   private createIndexSql(idx: IndexInfo, qualifiedTable: string, dialect?: SqlDialect): string {
+    // Oracle keeps a function-based index's expression in ALL_IND_EXPRESSIONS
+    // and puts a hidden `SYS_NC00006$` placeholder in ALL_IND_COLUMNS, which is
+    // what introspection reads. Emitting that name is ORA-00904 ("invalid
+    // identifier") — the index cannot be built from what we captured, so say so
+    // rather than shipping DDL that cannot run.
+    const hidden = idx.columns.find((c) => /^SYS_NC\d+\$$/i.test((c ?? '').trim()));
+    if (hidden) {
+      return `-- review: skip index ${idx.name} on ${qualifiedTable} — function-based index; its expression is not captured (column reads as ${hidden})`;
+    }
     // Quote the column names *before* handing them to a dialect hook: the hooks
     // build their own column list, so a hook-owning dialect (SQLite, SQL Server)
     // would otherwise emit `ON t (order id)` while the generic path below got
@@ -405,8 +414,11 @@ export class SqlGeneratorModule {
     if (s.increment !== undefined) opts += ` INCREMENT BY ${s.increment}`;
     if (s.minValue !== undefined) opts += ` MINVALUE ${s.minValue}`;
     if (s.maxValue !== undefined) opts += ` MAXVALUE ${s.maxValue}`;
-    opts += s.cycle ? ` CYCLE` : ` NO CYCLE`;
-    if (s.cache !== undefined) opts += s.cache > 0 ? ` CACHE ${s.cache}` : ` NO CACHE`;
+    // `NO CYCLE` on most engines, `NOCYCLE` on Oracle — the spaced form is
+    // ORA-03049 there, and it takes the dependent table and views down with it.
+    const no = dialect?.unspacedSequenceNoKeywords ? 'NO' : 'NO ';
+    opts += s.cycle ? ` CYCLE` : ` ${no}CYCLE`;
+    if (s.cache !== undefined) opts += s.cache > 0 ? ` CACHE ${s.cache}` : ` ${no}CACHE`;
     const createSql = `CREATE SEQUENCE ${name}${opts};`;
     return dialect?.wrapCreateSequence?.(name, createSql) ?? `CREATE SEQUENCE IF NOT EXISTS ${name}${opts};`;
   }
@@ -935,8 +947,9 @@ export class SqlGeneratorModule {
       if (s.increment !== undefined) alter += ` INCREMENT BY ${s.increment}`;
       if (s.minValue !== undefined) alter += ` MINVALUE ${s.minValue}`;
       if (s.maxValue !== undefined) alter += ` MAXVALUE ${s.maxValue}`;
-      alter += s.cycle ? ` CYCLE` : ` NO CYCLE`;
-      if (s.cache !== undefined) alter += s.cache > 0 ? ` CACHE ${s.cache}` : ` NO CACHE`;
+      const noAlter = dialect?.unspacedSequenceNoKeywords ? 'NO' : 'NO ';
+      alter += s.cycle ? ` CYCLE` : ` ${noAlter}CYCLE`;
+      if (s.cache !== undefined) alter += s.cache > 0 ? ` CACHE ${s.cache}` : ` ${noAlter}CACHE`;
       const alterSql = alter + `;`;
       statements.push(dialect.wrapAlterSequence?.(tableName, alterSql) ?? alterSql);
     } else if (obj.objectType === 'TYPE' && obj.sourceTable) {
@@ -1144,7 +1157,14 @@ export class SqlGeneratorModule {
     // Structural ADDED objects (TABLE, SEQUENCE, TYPE, ROLE) come before MODIFIED so
     // that new tables can be referenced by FK constraints added in ALTER steps.
     const addedStructural = diffs.filter((d) => d.status === 'ADDED' && !PROCEDURAL_TYPES.has(d.objectType));
-    for (const obj of this.sortAddedByDependency(addedStructural)) {
+    // Sequences, types and roles ahead of tables. `sortAddedByDependency` only
+    // knows about foreign keys, so a table whose column default calls a
+    // sequence (`DEFAULT order_seq.NEXTVAL`) was created before the sequence
+    // existed — ORA-02289 on Oracle, which then took out every view over that
+    // table. None of these can depend on a table, so first is always safe.
+    const supporting = addedStructural.filter((d) => d.objectType !== 'TABLE' && d.objectType !== 'MQT');
+    const tables = addedStructural.filter((d) => d.objectType === 'TABLE' || d.objectType === 'MQT');
+    for (const obj of [...supporting, ...this.sortAddedByDependency(tables)]) {
       const stmts = this.createObjectStatements(obj, dialect, m);
       steps.push({ objectName: obj.tableName, objectType: obj.objectType, action: 'CREATE', statements: stmts });
     }

packages/sql/src/providers/oracle/oracle.sql-dialect.ts

@@ -75,6 +75,12 @@ export const oracleSqlDialect: SqlDialect = {
     return c.identity ? ` GENERATED ${c.identityGeneration ?? 'ALWAYS'} AS IDENTITY` : '';
   },
 
+  // NOCYCLE / NOCACHE, not NO CYCLE / NO CACHE. Verified on Oracle 23 Free: the
+  // spaced form raises ORA-03049 ("SQL keyword 'NO' is not syntactically
+  // valid"), which failed the CREATE SEQUENCE and then cascaded — the table
+  // whose default calls the sequence (ORA-02289) and the views over it.
+  unspacedSequenceNoKeywords: true,
+
   // Oracle can't RESTART a sequence portably (RESTART START WITH is 18c+; older has no
   // equivalent), so skip the clause rather than emit invalid SQL.
   alterSequenceRestart(): string {
@@ -130,13 +136,20 @@ export const oracleSqlDialect: SqlDialect = {
   dropIndexStatement(indexName: string, qualifiedTable: string): string {
     const dot = qualifiedTable.indexOf('.');
     const prefix = dot >= 0 ? qualifiedTable.slice(0, dot + 1) : '';
-    return `DROP INDEX ${prefix}${indexName};`;
+    // Tolerant like the table/view/sequence drops above: an index can already
+    // be gone (dropped with its table, or never created because it is
+    // function-based and its expression was not captured), and ORA-01418 then
+    // failed the whole step. This hook gets no server version, so it always
+    // uses the SQLCODE guard, which is valid on every release.
+    return oracleDrop('INDEX', `${prefix}${indexName}`, -1418);
   },
 
   dropTriggerStatement(triggerName: string, qualifiedTable: string): string {
     const dot = qualifiedTable.indexOf('.');
     const prefix = dot >= 0 ? qualifiedTable.slice(0, dot + 1) : '';
-    return `DROP TRIGGER ${prefix}${triggerName};`;
+    // Same tolerance as the index drop: a trigger goes away with its table, and
+    // ORA-04080 on an already-absent one failed the step for no good reason.
+    return oracleDrop('TRIGGER', `${prefix}${triggerName}`, -4080);
   },
 
   createTriggerStatement(