packages/oracle/src/query-generator.js (5)

174-176: Consider aliasing the version column.

The query returns VERSION_FULL without an alias. Aliasing it to "version" would standardize the output and potentially avoid the need to override queryInterface.fetchDatabaseVersion in other parts of the codebase.

   versionQuery() {
-    return `SELECT VERSION_FULL FROM PRODUCT_COMPONENT_VERSION WHERE PRODUCT LIKE 'Oracle%'`;
+    return `SELECT VERSION_FULL AS "version" FROM PRODUCT_COMPONENT_VERSION WHERE PRODUCT LIKE 'Oracle%'`;
   }

---

1-1291: Migrate this entire file to TypeScript.

This file is implemented in JavaScript but the coding guidelines require: "Write all new implementations in TypeScript; avoid creating new .js files in src". The base OracleQueryGeneratorTypeScript already exists in packages/oracle/src/query-generator-typescript.internal.ts, so these Oracle-specific methods should be migrated into that TypeScript file rather than maintained in a separate JavaScript file.

Based on coding guidelines and learnings.

---

324-326: Fix array splice bug when removing processed indexes.

Splicing in forward order causes index misalignment. After the first splice, all subsequent indices shift down by one, making the remaining indices in idxToDelete point to wrong elements.

Apply this diff to fix by iterating in reverse:

-    idxToDelete.forEach(idx => {
-      options.indexes.splice(idx, 1);
-    });
+    for (let i = idxToDelete.length - 1; i >= 0; i--) {
+      options.indexes.splice(idxToDelete[i], 1);
+    }

---

741-747: Fix auto-increment detection across multiple rows.

allColumns[key] is overwritten for each row, so only the last row's value determines whether the column is treated as auto-increment. If rows have mixed null and non-null values for an auto-increment column, the detection will be incorrect unless the last row happens to be null.

The check at lines 773-777 partially catches this, but only when processing rows after allColumns[key] was already set to true. To properly enforce the invariant ("either all rows are null or all are non-null"), track both states:

+    const sawNull = {};
+    const sawNonNull = {};
+
     for (const fieldValueHash of fieldValueHashes) {
       forOwn(fieldValueHash, (value, key) => {
+        if (!fieldMappedAttributes[key]?.autoIncrement) {
+          allColumns[key] = false;
+          return;
+        }
+
+        const isNull = value === null;
+        sawNull[key] ||= isNull;
+        sawNonNull[key] ||= !isNull;
+
+        if (sawNull[key] && sawNonNull[key]) {
+          throw new Error(
+            'For an auto-increment column either all rows must be null or non-null; a mix is not allowed.',
+          );
+        }
+
-        allColumns[key] =
-          fieldMappedAttributes[key] &&
-          fieldMappedAttributes[key].autoIncrement === true &&
-          value === null;
+        allColumns[key] = sawNull[key];
       });
     }

---

947-950: Fix unreachable DOUBLE condition.

Line 949 compares attribute.type === 'DOUBLE' against a string, but attribute.type is a DataType instance, not a string. This condition is unreachable and will never execute. The pattern at lines 921, 933, and 940 correctly uses instanceof.

Change to:

     if (attribute.autoIncrement) {
       template = ' NUMBER(*,0) GENERATED BY DEFAULT ON NULL AS IDENTITY';
-    } else if (attribute.type && attribute.type === 'DOUBLE') {
-      template = attribute.type.toSql();
+    } else if (attribute.type instanceof DataTypes.DOUBLE) {
+      template = attribute.type.toSql();
     } else if (attribute.type) {

packages/core/test/integration/sequelize/query.test.js (2)

122-125: Verify Date object vs string handling for Oracle timestamps.

Oracle uses Date objects for createdAt and updatedAt while other dialects use strings. This differs from the insertQuery construction (lines 79-83) which uses string literals formatted by dateLiteral(). Please verify this discrepancy is intentional and reflects Oracle driver behavior for bind parameters vs literals.

Run the following script to check how Oracle bind parameters handle Date objects:

#!/bin/bash
# Search for Oracle date/timestamp handling in driver and serialization code
rg -nP --type=ts --type=js -C5 'oracle.*date|timestamp.*oracle' -g '!**/test/**' -g '**/oracle/**'

---

555-560: Unique constraint violations return generic DatabaseError for Oracle.

Oracle returns DatabaseError instead of UniqueConstraintError for duplicate key violations, preventing applications from catching and handling unique constraint errors specifically. This is a functional limitation that should either be fixed in the Oracle error classification logic or properly documented.

Verify whether Oracle error codes can be mapped to UniqueConstraintError:

#!/bin/bash
# Search for Oracle error classification logic
ast-grep --pattern 'class $_ extends DatabaseError {
  $$$
}'

# Find Oracle unique constraint error codes (typically ORA-00001)
rg -nP 'ORA-00001|unique.*constraint.*oracle|oracle.*unique.*constraint' -g '!**/test/**'

packages/oracle/src/query-generator.js (1)

464-471: Remove commented-out code.

Large blocks of commented code (lines 464-471 and 1114-1156) should be removed. If this code might be needed later, it's preserved in version control history. Keeping commented code in the source reduces readability and maintainability.

Also applies to: 1114-1156

packages/core/test/integration/sequelize/query.test.js (1)

892-918: Document why Oracle skips the duplicate named parameter test.

The test is excluded for Oracle (and db2) without explanation. If Oracle doesn't support reusing the same named bind parameter multiple times in a query, consider adding a comment or using a dialect.supports feature check instead of a direct dialect name check.

As per coding guidelines.

If this is a known Oracle limitation, add a clarifying comment:

-     if (!['db2', 'oracle'].includes(dialectName)) {
+     // Oracle and db2 don't support reusing named bind parameters in the same query
+     if (!['db2', 'oracle'].includes(dialectName)) {

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use Support.getTestDialectTeaser() for dialect-specific test descriptions

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use beforeEach/afterEach to sync with { force: true } and close Sequelize instances in tests

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : In tests, skip unsupported behavior with dialect.supports.featureName checks

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/dialect.ts : When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Co-locate TypeScript type definitions with their implementation files

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/*.js : Write all new implementations in TypeScript; avoid creating new .js files in src

Learnt from: SippieCup
Repo: sequelize/sequelize PR: 18051
File: packages/core/src/utils/undot.ts:177-177
Timestamp: 2025-11-08T03:05:56.951Z
Learning: In packages/core/src/utils/undot.ts, the defensive check "if (!Array.isArray(obj))" before numeric leaf assignment in setByPathArray is intentional to handle edge cases where table aliases or oddly-named columns produce single-segment numeric keys (e.g., "0").

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Prefer TypeScript for all new code; only modify existing .js files when necessary

packages/core/test/integration/json.test.ts (2)

68-70: Use this.skip() instead of early return to mark unsupported Oracle JSON as skipped

Right now this test just returns early on unsupported Oracle versions, which Mocha reports as a passing test instead of a skipped one, and the arrow function prevents access to the Mocha context (this.skip()).

You can make the intent explicit and align with the other Oracle JSON guards by switching to a regular function and calling this.skip():

-  it('should be able to store strings that require escaping', async () => {
-    if (dialect.name === 'oracle' && !(await isOracleJSONConstraintsSupported())) {
-      return;
-    }
+  it('should be able to store strings that require escaping', async function (this: Mocha.Context) {
+    if (dialect.name === 'oracle' && !(await isOracleJSONConstraintsSupported())) {
+      this.skip();
+    }

This makes unsupported Oracle configurations clearly show up as skipped rather than silently passing.

---

88-96: Simplify Oracle JSON version guard before hook

The logic here is correct and nicely addresses the earlier feedback about skipping only unsupported Oracle versions. The nested async IIFE is redundant, though; you can await the helper directly:

-  if (dialect.name === 'oracle') {
-    before(async function checkOracleVersionForJSONSupport(this: Mocha.Context) {
-      return (async () => {
-        if (!(await isOracleJSONConstraintsSupported())) {
-          this.skip();
-        }
-      })();
-    });
-  }
+  if (dialect.name === 'oracle') {
+    before(async function checkOracleVersionForJSONSupport(this: Mocha.Context) {
+      if (!(await isOracleJSONConstraintsSupported())) {
+        this.skip();
+      }
+    });
+  }

Behavior stays the same, but the hook is easier to read.

packages/core/test/integration/sequelize/query.test.js (1)

51-57: Inconsistent date literal format with inline usage.

The dateLiteral() helper uses to_date(...) for Oracle, but lines 533-534 and 572 use TO_TIMESTAMP(...) directly. Consider whether both formats are needed or if one should be standardized.

Apply this diff if you want to align the helper with the inline usage:

 const dateLiteral = str => {
   if (dialectName === 'oracle') {
-    return `to_date('${str}','YYYY-MM-DD HH24:MI:SS')`;
+    return `TO_TIMESTAMP('${str}','YYYY-MM-DD HH24:MI:SS')`;
   }
 
   return `'${str}'`;
 };

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : In tests, skip unsupported behavior with dialect.supports.featureName checks

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use Support.getTestDialectTeaser() for dialect-specific test descriptions

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/dialect.ts : When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/core/test/chai-extensions.d.ts : Use Chai assertions with the project’s custom extensions available in packages/core/test/chai-extensions.d.ts

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use beforeEach/afterEach to sync with { force: true } and close Sequelize instances in tests

packages/oracle/src/query.js (5)

24-38: Constructor still reads outFormat from raw options parameter.

This issue was flagged in a previous review but remains unresolved. Line 37 reads from the raw options parameter which can be undefined, causing a TypeError. The merged this.options (line 26) already handles options || {}, so you should use it consistently.

Apply this fix:

-    this.outFormat = options.outFormat || oracledb.OBJECT;
+    this.outFormat = this.options.outFormat || oracledb.OBJECT;

---

146-167: Transaction control flags don't take effect due to property name mismatch.

This critical issue was flagged in a previous review but remains unresolved. Lines 148, 155, and 162 set this.autocommit (lowercase), but all other code reads this.autoCommit (camelCase) at lines 45, 170, 180, 194, 202, 204, 253, 257. This prevents BEGIN TRANSACTION, SET AUTOCOMMIT ON, and SET AUTOCOMMIT OFF from affecting subsequent query execution.

Apply these fixes:

     if (this.sql.startsWith('BEGIN TRANSACTION')) {
-      this.autocommit = false;
+      this.autoCommit = false;
 
       // eslint-disable-next-line unicorn/no-useless-promise-resolve-reject
       return Promise.resolve();
     }
 
     if (this.sql.startsWith('SET AUTOCOMMIT ON')) {
-      this.autocommit = true;
+      this.autoCommit = true;
 
       // eslint-disable-next-line unicorn/no-useless-promise-resolve-reject
       return Promise.resolve();
     }
 
     if (this.sql.startsWith('SET AUTOCOMMIT OFF')) {
-      this.autocommit = false;
+      this.autoCommit = false;
 
       // eslint-disable-next-line unicorn/no-useless-promise-resolve-reject
       return Promise.resolve();
     }

---

339-435: Potential double-parse or type error for JSON attributes.

This issue was flagged in a previous review but remains unresolved. Lines 412-414 unconditionally call JSON.parse(value) for JSON types, then lines 426-429 still invoke the dialect parser for the same value. This risks:

• Double-parsing if the dialect parser also decodes JSON
• TypeError if value is a Buffer or already an object (Oracle stores JSON as BLOB per data-types-overrides.ts)

Prefer one of these solutions:

Option 1 (recommended): Let the dialect parser handle JSON entirely:

           if (modelDefinition.rawAttributes[key] && modelDefinition.rawAttributes[key].type) {
             let typeid = modelDefinition.rawAttributes[key].type.toLocaleString();
-            if (modelDefinition.rawAttributes[key].type.getDataTypeId() === 'JSON') {
-              value = JSON.parse(value);
-            }
 
             // For some types, the "name" of the type is returned with the length, we remove it

Option 2: Only parse strings and skip dialect parser for JSON:

+            let isJsonType = false;
             if (modelDefinition.rawAttributes[key].type.getDataTypeId() === 'JSON') {
+              isJsonType = true;
-              value = JSON.parse(value);
+              if (typeof value === 'string') {
+                value = JSON.parse(value);
+              }
             }
 
             // For some types, the "name" of the type is returned with the length, we remove it
             // For Boolean we skip this because BOOLEAN is mapped to CHAR(1) and we dont' want to
             // remove the (1) for BOOLEAN
             if (
               typeid.includes('(') &&
               modelDefinition.rawAttributes[key].type.getDataTypeId() !== 'BOOLEAN'
             ) {
               typeid = typeid.slice(0, typeid.indexOf('('));
             }
 
             const parser = this.sequelize.dialect.getParserForDatabaseDataType(typeid);
-            if (value !== null && parser) {
+            if (value !== null && parser && !isJsonType) {
               value = parser(value);
             }

Based on learnings.

---

545-559: isNewRecord flag remains inverted in upsert result.

This issue was flagged in a previous review but remains unresolved. Line 558 sets isNewRecord: data.isUpdate, which is 1 for updates and 0 for inserts—the opposite of the intended semantics. isNewRecord should be true for new records and false for existing ones.

Apply this fix:

-      result = [{ isNewRecord: data.isUpdate, value: data }, data.isUpdate == 0];
+      result = [{ isNewRecord: data.isUpdate == 0, value: data }, data.isUpdate == 0];

---

656-660: ORA-02443 error handling still has regex and constructor issues.

While the ORA-02291/02292 handling was fixed, the ORA-02443 case remains unresolved from the previous review. Line 657's regex has no capture group, so match[1] is undefined. Additionally, UnknownConstraintError expects an options object, not a string parameter.

Apply this fix:

     // ORA-02443: Cannot drop constraint  - nonexistent constraint
-    match = err.message.match(/ORA-02443/);
-    if (match && match.length > 0) {
-      return new UnknownConstraintError(match[1]);
+    match = err.message.match(/ORA-02443:[^-]*-\s*(.+?)(?:\s|$)/);
+    if (match && match.length > 1) {
+      return new UnknownConstraintError({
+        constraint: match[1].trim(),
+        cause: err,
+      });
     }

packages/core/test/integration/error.test.ts (4)

553-563: Oracle ValidationError.errors parity with other dialects

Treating Oracle like db2 (i.e. ValidationError.errors being empty for bulk unique violations) is acceptable for initial support, but it does mean Oracle exposes less structured info than most other dialects here. Consider adding a TODO to parse the ORA-00001 details into error.errors later for parity, once the dialect stabilizes.

Also applies to: 633-647

---

606-608: Harden Oracle unique-constraint regex for index-name matching

The generic ORA‑00001 check is fine, but the index‑specific one:

/ORA-00001: unique constraint \(.*.users_username_unique\) violated/

relies on exact casing and an unescaped . before users_username_unique. To make this more resilient to schema prefixes & case differences, you could tighten it to something like:

- /ORA-00001: unique constraint \(.*.users_username_unique\) violated/
+ /ORA-00001: unique constraint \((?:.+\.)?users_username_unique\) violated/i

This still asserts the right constraint name while tolerating driver/DDL variations.

Also applies to: 682-686

---

763-769: Oracle FK errors expose less metadata than other dialects

For Oracle FK violations you currently assert error.table is undefined and error.fields is null, whereas other dialects generally surface at least table or fields. Oracle’s ORA‑02291/02292 messages contain the constraint name, so in the future it should be possible to extract table/column info for better parity; if you plan to improve the Oracle error mapping, these expectations will need to be relaxed or updated accordingly.

Also applies to: 846-852

---

896-899: Consider aligning Oracle duplicate-constraint mapping with UnknownConstraintError

For duplicate constraint names you assert a bare DatabaseError on Oracle, while MariaDB/MySQL/MSSQL/sqlite3 surface an AggregateError that includes an UnknownConstraintError with constraint and table. That’s a divergence in the cross‑dialect error contract: user code catching UnknownConstraintError won’t work on Oracle here. If feasible, consider mapping ORA‑02264 into the same UnknownConstraintError shape so behavior is consistent across dialects.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : In tests, skip unsupported behavior with dialect.supports.featureName checks

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use Support.getTestDialectTeaser() for dialect-specific test descriptions

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

packages/oracle/src/query-generator.js (4)

3-1214: This new dialect implementation should ideally live in TypeScript, not a .js file.

Per the repo guidelines, new implementations under packages/**/src/** should be written in TypeScript, and dialect SQL should go into query-generator-typescript.internal.ts (with a thin JS wrapper only if absolutely needed). This file contains substantial new logic (DDL, DML, JSON helpers, bind handling) that would benefit from typings, and it also triggers lint/parser issues in tools expecting ESM TS.

Consider migrating the concrete methods from this file into OracleQueryGeneratorTypeScript and keeping this JS file as a minimal public wrapper (or removing it entirely if consumers can import the TS‑built artifact directly).

---

254-323: Fix unique‑index → unique‑constraint handling: splice logic and unique type assumptions.

Two issues in the createTableQuery index‑to‑constraint conversion block:

1. Forward splice over idxToDelete can remove wrong indexes.
   Splicing options.indexes in ascending index order shifts subsequent indices; later deletions may target the wrong element or no element at all. This leaves some indexes in options.indexes even though they were meant to be converted to constraints.

2. index.unique.length assumes unique is not false.
   For { unique: false }, the condition index.unique === true || (index.unique.length > 0 && index.unique !== false) still evaluates the length access on a boolean, which will throw at runtime.

A safe fix is to normalize unique to a boolean and remove indexes in reverse order:

-    if (options && options.indexes && options.indexes.length > 0) {
-      const idxToDelete = [];
+    if (options && options.indexes && options.indexes.length > 0) {
+      const idxToDelete: number[] = [];
       options.indexes.forEach((index, idx) => {
-        if (
-          'unique' in index &&
-          (index.unique === true || (index.unique.length > 0 && index.unique !== false))
-        ) {
+        const unique =
+          typeof index.unique === 'boolean'
+            ? index.unique
+            : Boolean(index.unique);
+
+        if ('unique' in index && unique) {
           // If unique index, transform to unique constraint on column
           const fields = index.fields.map(field => {
             if (typeof field === 'string') {
               return field;
             }
 
             return field.attribute;
           });
@@
-      idxToDelete.forEach(idx => {
-        options.indexes.splice(idx, 1);
-      });
+      // delete from the end to avoid index shifting
+      for (let i = idxToDelete.length - 1; i >= 0; i--) {
+        options.indexes.splice(idxToDelete[i], 1);
+      }

This preserves the intended behavior and avoids index‑shift bugs.

---

682-727: Auto‑increment detection in bulkInsertQuery is incorrect when last row is non‑null.

The allColumns map is recomputed for every row:

for (const fieldValueHash of fieldValueHashes) {
  forOwn(fieldValueHash, (value, key) => {
    allColumns[key] =
      fieldMappedAttributes[key] &&
      fieldMappedAttributes[key].autoIncrement === true &&
      value === null;
  });
}

This means allColumns[key] only reflects the last row’s value for that column. Later, when building tuples, mixed null / non‑null values are only rejected if the last row is null; if the last row is non‑null, a mix of null and non‑null silently passes and the column is treated as a regular in‑bind.

To enforce the intended invariant (“either all rows are null or all are non‑null for auto‑increment columns”), track both “saw null” and “saw non‑null” per column:

-    const allColumns = {};
+    const allColumns: Record<string, boolean> = {};
@@
-    for (const fieldValueHash of fieldValueHashes) {
-      forOwn(fieldValueHash, (value, key) => {
-        allColumns[key] =
-          fieldMappedAttributes[key] &&
-          fieldMappedAttributes[key].autoIncrement === true &&
-          value === null;
-      });
-    }
+    const sawNull: Record<string, boolean> = {};
+    const sawNonNull: Record<string, boolean> = {};
+
+    for (const fieldValueHash of fieldValueHashes) {
+      forOwn(fieldValueHash, (value, key) => {
+        if (!fieldMappedAttributes[key]?.autoIncrement) {
+          allColumns[key] = false;
+          return;
+        }
+
+        const isNull = value === null;
+        sawNull[key] ||= isNull;
+        sawNonNull[key] ||= !isNull;
+
+        if (sawNull[key] && sawNonNull[key]) {
+          throw new Error(
+            'For an auto-increment column either all rows must be null or non-null; a mix is not allowed.',
+          );
+        }
+
+        // true means "use DEFAULT / outbind"
+        allColumns[key] = sawNull[key];
+      });
+    }

This ensures you either generate DEFAULT for that column in every row or treat it as a regular bound column, and you consistently reject mixed sets.

---

848-955: Unreachable / incorrect DOUBLE handling and type checks in attributeToSQL.

The branch:

} else if (attribute.type && attribute.type === 'DOUBLE') {
  template = attribute.type.toSql();
}

is problematic:

• In normal usage, attribute.type is a DataTypes instance, not the string 'DOUBLE', so this condition never matches.
• If attribute.type were the string 'DOUBLE', calling toSql() would throw.

Oracle DOUBLE columns will currently go through the generic else if (attribute.type) path, so this is effectively dead or buggy code.

A safer approach is either to:

• Remove this branch entirely and let the generic attribute.type.toString() path handle DOUBLE, or
• Check for the actual type instance, e.g.:

-    } else if (attribute.type && attribute.type === 'DOUBLE') {
-      template = attribute.type.toSql();
-    } else if (attribute.type) {
+    } else if (attribute.type instanceof DataTypes.DOUBLE) {
+      template = attribute.type.toSql();
+    } else if (attribute.type) {

which aligns with the earlier ENUM / JSON / BOOLEAN checks.

packages/oracle/src/query-generator-typescript.internal.ts (2)

140-157: removeConstraintQuery generates invalid Oracle syntax for ifExists.

For constraints starting with sys, this override emits:

ALTER TABLE <table> DROP CONSTRAINT IF EXISTS <name> [CASCADE]

Oracle’s ALTER TABLE ... DROP CONSTRAINT does not support IF EXISTS, so any call with options.ifExists === true will fail at runtime.

To fix this:

• Either reject ifExists for Oracle by validating options before building SQL, e.g.:

+    if (options) {
+      // Oracle does not support IF EXISTS for DROP CONSTRAINT
+      rejectInvalidOptions(
+        'removeConstraintQuery',
+        this.dialect,
+        /* REMOVE_CONSTRAINT_QUERY_SUPPORTABLE_OPTIONS */ new Set(['ifExists', 'cascade']),
+        /* supportedOptions */ new Set(['cascade']),
+        options,
+      );
+    }
+
     if (constraintName.startsWith('sys')) {
       return joinSQLFragments([
         'ALTER TABLE',
         this.quoteTable(tableName),
         'DROP CONSTRAINT',
-        options?.ifExists ? 'IF EXISTS' : '',
         constraintName,
         options?.cascade ? 'CASCADE' : '',
       ]);
     }

• Or, if you truly need an “if exists” behavior, wrap the DROP in a PL/SQL block that checks the data dictionary before executing.

Right now, any use of ifExists here will be broken.

Does Oracle SQL support `ALTER TABLE ... DROP CONSTRAINT IF EXISTS` syntax, and what is the recommended pattern to conditionally drop a constraint if it exists?

---

379-408: Use Oracle’s COMMIT / ROLLBACK syntax instead of T‑SQL forms.

commitTransactionQuery and rollbackTransactionQuery currently return:

return 'COMMIT TRANSACTION';
...
return 'ROLLBACK TRANSACTION';

These are SQL Server / T‑SQL forms and are not valid Oracle SQL; Oracle expects COMMIT (optionally COMMIT WORK) and ROLLBACK. Because Oracle does not enable supports.connectionTransactionMethods by default, these methods will be called and produce invalid SQL.

You should either:

• Return plain Oracle statements:

  commitTransactionQuery() {
-    return 'COMMIT TRANSACTION';
+    return 'COMMIT';
  }

  rollbackTransactionQuery(): string {
    if (this.dialect.supports.connectionTransactionMethods) {
      throw new Error(
        `rollbackTransactionQuery is not supported by the ${this.dialect.name} dialect.`,
      );
    }

-    return 'ROLLBACK TRANSACTION';
+    return 'ROLLBACK';
  }

• Or, if Oracle is always meant to use connection‑level transaction control, set supports.connectionTransactionMethods = true for this dialect and make these methods throw unconditionally.

As written, they are very likely to generate invalid SQL.

What are the valid SQL statements for committing and rolling back a transaction in Oracle Database, and does Oracle accept `COMMIT TRANSACTION` / `ROLLBACK TRANSACTION`?

packages/oracle/src/dialect.ts (3)

88-90: Documentation URL doesn't match minimum database version.

Line 88 links to Oracle 23 documentation, but line 90 sets minimumDatabaseVersion to '18.0.0'. Oracle 23 introduced features unavailable in 18.0.0 (native JSON SQL type, SQL Domains, spatial enhancements).

Either update the URL to Oracle 18 documentation or increase the minimum version to 23.0.0 if Oracle 23-specific features are required.

---

109-111: Runtime error when replication config is absent.

this.sequelize.options.replication.write.username?.toUpperCase() will throw if replication or write is undefined (common in non-replication setups), because property access occurs before optional chaining on username.

Apply this diff to handle both replication and direct username safely:

 getDefaultSchema(): string {
-  return this.sequelize.options.replication.write.username?.toUpperCase() ?? '';
+  const replicationUser = this.sequelize.options.replication?.write?.username;
+  const directUser = this.sequelize.options.username;
+  const username = replicationUser ?? directUser;
+  return username?.toUpperCase() ?? '';
 }

---

135-182: Fix typos in validation comments.

The date validation logic is thorough and security-conscious, but there are two typos:

• Line 141: "indicated y" should be "indicated by"
• Line 146: "TIMESTAM_TZ" should be "TIMESTAMP_TZ"

Apply this diff:

     // Validate that the split result has exactly three parts (function name, parameters, and an empty string)
-    // and that there are no additional SQL commands after the function call (indicated y the last empty string).
+    // and that there are no additional SQL commands after the function call (indicated by the last empty string).
     if (splitVal.length !== 3 || splitVal[2] !== '') {
       throw new Error('Invalid SQL function call.'); // Error if function call has unexpected format
     }
 
-    // Extract the function name (either 'TO_TIMESTAM_TZ' or 'TO_DATE') and the contents inside the parentheses
+    // Extract the function name (either 'TO_TIMESTAMP_TZ' or 'TO_DATE') and the contents inside the parentheses
     const functionName = splitVal[0].trim(); // Function name should be 'TO_TIMESTAMP_TZ' or 'TO_DATE'

packages/oracle/src/query-interface-typescript.internal.ts (1)

13-26: Avoid overriding fetchDatabaseVersion by aligning Oracle’s versionQuery output.

This override exists only because Oracle returns VERSION_FULL. If you instead alias it to version in OracleQueryGeneratorTypeScript.versionQuery (e.g. SELECT VERSION_FULL AS version ...), you can drop this method and rely on the base AbstractQueryInterfaceTypeScript.fetchDatabaseVersion (including its runtime assertion on the version field). That keeps Oracle in line with other dialects and removes an extra Oracle‑specific code path here.

packages/core/src/abstract-dialect/query-interface-typescript.ts (1)

261-279: Good use of a dialect flag to control concurrent FK drops.

The new concurrentDropConstraints guard makes FK removal concurrent only for dialects that explicitly support it, while keeping the previous sequential behavior as the default. This limits deadlock/timeout risk and keeps behavior configurable per dialect.

Ensure each dialect sets supports.dropTable.concurrentDropConstraints appropriately (e.g. conservative false for engines known to deadlock under concurrent DDL).

packages/oracle/src/dialect.ts (1)

16-16: Consider using a type alias for the empty interface.

An empty interface is equivalent to {}. If this is a placeholder for future options, consider using a type alias for better semantics.

Apply this diff:

-export interface OracleDialectOptions {}
+export type OracleDialectOptions = Record<string, never>;

Alternatively, if options will be added soon, the empty interface is acceptable as a placeholder.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/dialect.ts : When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Co-locate TypeScript type definitions with their implementation files

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/*.js : Write all new implementations in TypeScript; avoid creating new .js files in src

Learnt from: SippieCup
Repo: sequelize/sequelize PR: 18051
File: packages/core/src/utils/undot.ts:177-177
Timestamp: 2025-11-08T03:05:56.951Z
Learning: In packages/core/src/utils/undot.ts, the defensive check "if (!Array.isArray(obj))" before numeric leaf assignment in setByPathArray is intentional to handle edge cases where table aliases or oddly-named columns produce single-segment numeric keys (e.g., "0").

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Prefer TypeScript for all new code; only modify existing .js files when necessary

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : In tests, skip unsupported behavior with dialect.supports.featureName checks

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use Support.getTestDialectTeaser() for dialect-specific test descriptions

packages/oracle/src/query-generator.js (4)

1-1237: This file should be implemented in TypeScript.

The coding guidelines require all new implementations to be written in TypeScript. This was already flagged in previous reviews with a comprehensive migration plan, but the file remains as JavaScript.

Based on coding guidelines.

---

320-322: Fix array splice bug when removing processed indexes.

Splicing in forward order within the loop causes index misalignment. After the first splice, all subsequent indices shift down, making the remaining indices in idxToDelete point to the wrong elements.

Apply this diff to fix by iterating in reverse order:

-    idxToDelete.forEach(idx => {
-      options.indexes.splice(idx, 1);
-    });
+    // Splice in reverse order to avoid index shifting issues
+    for (let i = idxToDelete.length - 1; i >= 0; i--) {
+      options.indexes.splice(idxToDelete[i], 1);
+    }

---

713-720: Fix auto-increment detection to validate across all rows.

allColumns[key] is overwritten for each row, so it only reflects the last row's value. A mix of null and non-null values for an auto-increment column is only detected when the last row is null. If the last row is non-null, the mix passes silently.

Apply this diff to track both states and enforce the invariant:

+    const sawNull = {};
+    const sawNonNull = {};
+
     for (const fieldValueHash of fieldValueHashes) {
       forOwn(fieldValueHash, (value, key) => {
-        allColumns[key] =
-          fieldMappedAttributes[key] &&
-          fieldMappedAttributes[key].autoIncrement === true &&
-          value === null;
+        if (!fieldMappedAttributes[key]?.autoIncrement) {
+          allColumns[key] = false;
+          return;
+        }
+
+        const isNull = value === null;
+        sawNull[key] ||= isNull;
+        sawNonNull[key] ||= !isNull;
+
+        if (sawNull[key] && sawNonNull[key]) {
+          throw new Error(
+            'For an auto-increment column either all rows must be null or non-null; a mix is not allowed.',
+          );
+        }
+
+        allColumns[key] = sawNull[key];
       });
     }

---

921-922: Fix unreachable DOUBLE condition.

Line 921 compares attribute.type === 'DOUBLE' against a string, but attribute.type is a DataType class instance. This condition is unreachable dead code.

Apply this diff to use instanceof check (consistent with ENUM at line 893, JSON at line 905, and BOOLEAN at line 912):

-    } else if (attribute.autoIncrement) {
+    if (attribute.autoIncrement) {
       template = ' NUMBER(*,0) GENERATED BY DEFAULT ON NULL AS IDENTITY';
-    } else if (attribute.type && attribute.type === 'DOUBLE') {
-      template = attribute.type.toSql();
-    } else if (attribute.type) {
+    } else if (attribute.type instanceof DataTypes.DOUBLE) {
+      template = attribute.type.toSql();
+    } else if (attribute.type) {

packages/oracle/src/query-generator.js (2)

436-443: Remove commented-out code.

Commented-out code should be removed and tracked in version control history instead. If this implementation is needed later, add a TODO comment with context.

Apply this diff:

-  // addConstraintQuery(tableName, options) {
-  //   options = options || {};
-
-  //   const constraintSnippet = this.getConstraintSnippet(tableName, options);
-
-  //   tableName = this.quoteTable(tableName);
-  //   return `ALTER TABLE ${tableName} ADD ${constraintSnippet};`;
-  // }
-
   addColumnQuery(table, key, dataType, options) {

---

1060-1102: Remove large commented-out code block.

This 42-line commented-out implementation should be removed. If the functionality is needed, create a TODO comment with an issue reference.

Apply this diff:

-  // handleSequelizeMethod(smth, tableName, factory, options, prepend) {
-  //   let str;
-  //   if (smth instanceof Utils.Json) {
-  //     // Parse nested object
-  //     if (smth.conditions) {
-  //       const conditions = this.parseConditionObject(smth.conditions).map(condition =>
-  //         `${this.jsonPathExtractionQuery(condition.path[0], _.tail(condition.path))} = '${condition.value}'`
-  //       );
-  //
-  //       return conditions.join(' AND ');
-  //     }
-  //     if (smth.path) {
-  //
-  //       // Allow specifying conditions using the sqlite json functions
-  //       if (this._checkValidJsonStatement(smth.path)) {
-  //         str = smth.path;
-  //       } else {
-  //         // Also support json property accessors
-  //         const paths = _.toPath(smth.path);
-  //         const column = paths.shift();
-  //         str = this.jsonPathExtractionQuery(column, paths);
-  //       }
-  //       if (smth.value) {
-  //         str += util.format(' = %s', this.escape(smth.value));
-  //       }
-  //
-  //       return str;
-  //     }
-  //   }
-  //   if (smth instanceof Utils.Cast) {
-  //     if (smth.val instanceof Utils.SequelizeMethod) {
-  //       str = this.handleSequelizeMethod(smth.val, tableName, factory, options, prepend);
-  //       if (smth.type === 'boolean') {
-  //         str = `(CASE WHEN ${str}='true' THEN 1 ELSE 0 END)`;
-  //         return `CAST(${str} AS NUMBER)`;
-  //       } if (smth.type === 'timestamptz' && /json_value\(/.test(str)) {
-  //         str = str.slice(0, -1);
-  //         return `${str} RETURNING TIMESTAMP WITH TIME ZONE)`;
-  //       }
-  //     }
-  //   }
-  //   return super.handleSequelizeMethod(smth, tableName, factory, options, prepend);
-  // }
-
   _checkValidJsonStatement(stmt) {

packages/oracle/src/query-interface.d.ts (1)

1-8: Consider adding explicit upsert method typing to the type declaration file.

The type file inherits the upsert method from AbstractQueryInterface through the empty class body, which is technically correct. However, since the implementation file contains custom Oracle-specific upsert logic (handling upsertKeys, constraints, and unique indexes), explicitly declaring the method signature in the .d.ts file would improve type clarity and IDE support for library consumers:

async upsert(
  tableName: string | { tableName: string },
  insertValues: object,
  updateValues: object,
  where: object,
  options: object
): Promise<boolean | number | undefined>;

This makes the contract explicit and improves discoverability of the Oracle dialect's upsert behavior.

packages/core/test/integration/model.test.js (1)

475-667: Oracle index expectations look consistent, but test is position‑sensitive

The new Oracle branch that assigns idx1 = args[1], idx2 = args[2], idx3 = args[3] and asserts only on fields (not type) matches how Oracle is expected to surface these indexes (1 composite + 2 single‑column). This is consistent with the rest of the suite.

The only downside is the usual brittleness around relying on args[N] ordering. If Oracle ever changes index ordering (e.g., by name), this test could break even though behavior is correct. Long‑term, consider locating indexes by name (e.g., via args.find(idx => idx.name === 'a_b_uniq')) instead of array position, similar to what you already do in some other tests.

packages/core/test/unit/sql/update.test.js (1)

90-90: Remove redundant Oracle expectation.

The Oracle expectation is identical to the default expectation after quote character substitution. Per the comment at line 10, the expectsql helper automatically replaces [] with dialect-specific quote characters (double quotes for Oracle). Adding an explicit Oracle expectation here is redundant and increases maintenance overhead without providing value.

Apply this diff to remove the redundant line:

             db2: 'SELECT * FROM FINAL TABLE (UPDATE "users" SET "user_name"=$sequelize_1 WHERE "id" = $sequelize_2);',
             snowflake: 'UPDATE "users" SET "user_name"=$sequelize_1 WHERE "id" = $sequelize_2',
-            oracle: `UPDATE "users" SET "user_name"=$sequelize_1 WHERE "id" = $sequelize_2`,
             default: 'UPDATE `users` SET `user_name`=$sequelize_1 WHERE `id` = $sequelize_2',

Note: This addresses the past review comment asking "What's the difference with the default?"

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/dialect.ts : When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use Support.getTestDialectTeaser() for dialect-specific test descriptions

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : In tests, skip unsupported behavior with dialect.supports.featureName checks

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Co-locate TypeScript type definitions with their implementation files

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/*.js : Write all new implementations in TypeScript; avoid creating new .js files in src

Learnt from: SippieCup
Repo: sequelize/sequelize PR: 18051
File: packages/core/src/utils/undot.ts:177-177
Timestamp: 2025-11-08T03:05:56.951Z
Learning: In packages/core/src/utils/undot.ts, the defensive check "if (!Array.isArray(obj))" before numeric leaf assignment in setByPathArray is intentional to handle edge cases where table aliases or oddly-named columns produce single-segment numeric keys (e.g., "0").

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Prefer TypeScript for all new code; only modify existing .js files when necessary

packages/oracle/src/query-generator-typescript.internal.ts (2)

145-154: Remove IF EXISTS from Oracle constraint drop—still not supported.

Line 150 still emits IF EXISTS when options?.ifExists is true, but Oracle does not support IF EXISTS in DROP CONSTRAINT clauses. This will cause a runtime SQL error (ORA-11557).

Either reject ifExists as unsupported (validate via rejectInvalidOptions before emitting SQL), or implement conditional dropping via PL/SQL (query catalog and conditionally execute DROP).

---

396-408: Fix transaction SQL to use Oracle syntax.

Lines 397 and 407 return 'COMMIT TRANSACTION' and 'ROLLBACK TRANSACTION', which are SQL Server/T-SQL forms. Oracle uses 'COMMIT' and 'ROLLBACK'.

Since the Oracle dialect does not set connectionTransactionMethods: true, these methods will be called and produce invalid SQL at runtime.

Apply this diff:

  commitTransactionQuery() {
-   return 'COMMIT TRANSACTION';
+   return 'COMMIT';
  }

  rollbackTransactionQuery(): string {
    if (this.dialect.supports.connectionTransactionMethods) {
      throw new Error(
        `rollbackTransactionQuery is not supported by the ${this.dialect.name} dialect.`,
      );
    }

-   return 'ROLLBACK TRANSACTION';
+   return 'ROLLBACK';
  }

packages/oracle/src/query-generator.js (3)

320-322: Fix array splice to avoid index shifting.

Splicing options.indexes by iterating idxToDelete in forward order causes index misalignment. After the first splice, all subsequent indices shift down, making remaining indices in idxToDelete point to wrong elements.

Apply this diff to splice in reverse order:

-    idxToDelete.forEach(idx => {
-      options.indexes.splice(idx, 1);
-    });
+    // Splice in reverse order to avoid index shifting
+    for (let i = idxToDelete.length - 1; i >= 0; i--) {
+      options.indexes.splice(idxToDelete[i], 1);
+    }

---

690-696: Fix auto-increment detection across multiple rows.

allColumns[key] is overwritten for each row (line 692-695), so only the last row's value is checked. A mix of null and non-null values for an auto-increment column is only detected when the last row is null. If the last row is non-null, the mix passes silently.

Track both "sawNull" and "sawNonNull" per column and throw if both are observed.

Apply this diff:

+    const sawNull = {};
+    const sawNonNull = {};
+
     for (const fieldValueHash of fieldValueHashes) {
       forOwn(fieldValueHash, (value, key) => {
-        allColumns[key] =
-          fieldMappedAttributes[key] &&
-          fieldMappedAttributes[key].autoIncrement === true &&
-          value === null;
+        if (!fieldMappedAttributes[key]?.autoIncrement) {
+          allColumns[key] = false;
+          return;
+        }
+
+        const isNull = value === null;
+        sawNull[key] ||= isNull;
+        sawNonNull[key] ||= !isNull;
+
+        if (sawNull[key] && sawNonNull[key]) {
+          throw new Error(
+            'For an auto-increment column either all rows must be null or non-null; a mix is not allowed.',
+          );
+        }
+
+        allColumns[key] = sawNull[key];
       });
     }

---

898-899: Fix unreachable DOUBLE condition.

Line 898 compares attribute.type === 'DOUBLE' against a string, but attribute.type is a DataType instance. This condition is unreachable and will never execute.

Change to use instanceof:

-    } else if (attribute.autoIncrement) {
+    if (attribute.autoIncrement) {
       template = ' NUMBER(*,0) GENERATED BY DEFAULT ON NULL AS IDENTITY';
-    } else if (attribute.type && attribute.type === 'DOUBLE') {
-      template = attribute.type.toSql();
-    } else if (attribute.type) {
+    } else if (attribute.type instanceof DataTypes.DOUBLE) {
+      template = attribute.type.toSql();
+    } else if (attribute.type) {

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/dialect.ts : When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Co-locate TypeScript type definitions with their implementation files

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/*.js : Write all new implementations in TypeScript; avoid creating new .js files in src

Learnt from: SippieCup
Repo: sequelize/sequelize PR: 18051
File: packages/core/src/utils/undot.ts:177-177
Timestamp: 2025-11-08T03:05:56.951Z
Learning: In packages/core/src/utils/undot.ts, the defensive check "if (!Array.isArray(obj))" before numeric leaf assignment in setByPathArray is intentional to handle edge cases where table aliases or oddly-named columns produce single-segment numeric keys (e.g., "0").

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Prefer TypeScript for all new code; only modify existing .js files when necessary

packages/oracle/src/dialect.ts (3)

86-101: Address the documentation URL and minimum version mismatch.

The dataTypesDocumentationUrl points to Oracle Database 23 documentation, while minimumDatabaseVersion is set to '18.0.0'. Oracle 23 introduced features not available in Oracle 18 (native JSON SQL type, SQL Domains, spatial enhancements). This mismatch may confuse users running Oracle 18 who reference the Oracle 23 documentation.

Consider either:

• Updating dataTypesDocumentationUrl to reference Oracle 18 documentation to align with the minimum version, or
• Increasing minimumDatabaseVersion to '23.0.0' if Oracle 23 features are required

---

109-111: CRITICAL: Fix runtime error in getDefaultSchema for non-replication setups.

getDefaultSchema will throw a runtime error when replication or write is undefined, which is the default for non-replication configurations. The optional chaining on username is evaluated only after the property access to replication.write succeeds, so it doesn't prevent the error.

Apply this diff to safely handle both replication and non-replication configs:

 getDefaultSchema(): string {
-  return this.sequelize.options.replication.write.username?.toUpperCase() ?? '';
+  const replicationUser = this.sequelize.options.replication?.write?.username;
+  const directUser = this.sequelize.options.username;
+
+  const username = replicationUser ?? directUser;
+
+  return username?.toUpperCase() ?? '';
 }

---

135-182: Fix typos in comments.

Two minor typos remain in the validation comments:

• Line 141: "indicated y the last empty string" → "indicated by the last empty string"
• Line 146: Comment references "TIMESTAM_TZ" → should be "TIMESTAMP_TZ"

Apply this diff:

     // Validate that the split result has exactly three parts (function name, parameters, and an empty string)
-    // and that there are no additional SQL commands after the function call (indicated y the last empty string).
+    // and that there are no additional SQL commands after the function call (indicated by the last empty string).
     if (splitVal.length !== 3 || splitVal[2] !== '') {
       throw new Error('Invalid SQL function call.'); // Error if function call has unexpected format
     }

-    // Extract the function name (either 'TO_TIMESTAMP_TZ' or 'TO_DATE') and the contents inside the parentheses
+    // Extract the function name (either 'TO_TIMESTAMP_TZ' or 'TO_DATE') and the contents inside the parentheses
     const functionName = splitVal[0].trim(); // Function name should be 'TO_TIMESTAMP_TZ' or 'TO_DATE'

packages/oracle/src/dialect.ts (1)

16-16: Convert empty interface to type alias or add properties.

The empty interface is flagged by static analysis and is equivalent to {}. Consider using a type alias instead for clarity, or add Oracle-specific dialect options if needed.

Apply this diff to convert to a type alias:

-export interface OracleDialectOptions {}
+export type OracleDialectOptions = Record<string, never>;

Or, if Oracle-specific options are planned, add a comment:

-export interface OracleDialectOptions {}
+// TODO: Add Oracle-specific dialect options as needed
+export interface OracleDialectOptions {}

Based on static analysis hints.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-generator.{js,ts} : Implement dialect feature SQL in query-generator.ts (preferred) or legacy query-generator.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/dialect.ts : When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/src/**/query-generator.{js,ts} : When modifying query generation, update both the base query generator and all dialect-specific implementations

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/*/src/query-interface.{js,ts} : Add schema operations for new features in query-interface.ts (preferred) or legacy query-interface.js

Learnt from: CR
Repo: sequelize/sequelize PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-29T19:49:24.715Z
Learning: Applies to packages/**/test/**/*.test.{js,ts} : Use Support.getTestDialectTeaser() for dialect-specific test descriptions

packages/core/test/unit/query-generator/create-schema-query.test.ts (2)

16-16: Role quoting and idempotency issues remain in Oracle CREATE SCHEMA expectation.

The password security concern from the previous review has been resolved (now using IDENTIFIED EXTERNALLY). However, two issues remain:

1. Incorrect role quoting: Oracle predefined role names like CONNECT should be granted without quotes. The expectation should use GRANT CONNECT TO "mySchema" instead of GRANT "CONNECT" TO "mySchema" (and similarly for other system privileges).

2. Partial idempotency: The grants only execute when NOT USER_FOUND (i.e., user newly created). If the user already exists but lacks required permissions, no grants are re-applied, potentially leaving the user in an incomplete state.

These issues should be addressed in the implementation (packages/oracle/src/query-generator-typescript.internal.ts) and the test expectation updated accordingly.

---

128-132: Incomplete list of invalid options for Oracle in combined options test.

The Oracle expectation only lists 3 invalid options but should list 6. Based on the individual option tests:

• Line 65: comment is only supported by Snowflake, so Oracle should reject it
• Line 74: ifNotExists is explicitly rejected by Oracle
• Line 84: replace is only supported by MariaDB and Snowflake, so Oracle should reject it

🔎 Apply this diff to add the missing invalid options:

 oracle: buildInvalidOptionReceivedError('createSchemaQuery', dialectName, [
   'authorization',
   'charset',
   'collate',
+  'comment',
+  'ifNotExists',
+  'replace',
 ]),

packages/oracle/src/query-generator-typescript.internal.ts (2)

396-408: Fix transaction SQL to use Oracle syntax.

Lines 397 and 407 return T-SQL syntax (COMMIT TRANSACTION / ROLLBACK TRANSACTION), which Oracle does not support. Oracle requires COMMIT and ROLLBACK without the TRANSACTION keyword. Since the Oracle dialect does not set connectionTransactionMethods: true, these methods will be invoked and produce runtime SQL errors.

🔎 Apply this diff to fix the SQL syntax:

  commitTransactionQuery() {
-   return 'COMMIT TRANSACTION';
+   return 'COMMIT';
  }

  rollbackTransactionQuery(): string {
    if (this.dialect.supports.connectionTransactionMethods) {
      throw new Error(
        `rollbackTransactionQuery is not supported by the ${this.dialect.name} dialect.`,
      );
    }

-   return 'ROLLBACK TRANSACTION';
+   return 'ROLLBACK';
  }

---

140-157: IF EXISTS in removeConstraintQuery is still present despite being marked resolved.

The past review comment flagged that Oracle does not support IF EXISTS in DROP CONSTRAINT clauses (line 150), marked as "✅ Addressed in commit 8454ae2." However, the code still emits IF EXISTS when options?.ifExists is true for sys-prefixed constraints, which will cause ORA-11557 runtime errors.

Either validate and reject the ifExists option before building SQL, or implement PL/SQL conditional logic querying the data dictionary to check constraint existence.

packages/oracle/src/query-generator.js (3)

320-322: Fix index shifting bug when splicing options.indexes.

Splicing array elements while iterating idxToDelete in forward order causes index misalignment. After the first splice, remaining indices point to wrong elements. This results in incorrect indexes being removed.

🔎 Apply this diff to fix (reverse iteration):

-    idxToDelete.forEach(idx => {
-      options.indexes.splice(idx, 1);
-    });
+    // Splice in reverse order to avoid index shifting
+    for (let i = idxToDelete.length - 1; i >= 0; i--) {
+      options.indexes.splice(idxToDelete[i], 1);
+    }

Or use filter to rebuild the array:

-    idxToDelete.forEach(idx => {
-      options.indexes.splice(idx, 1);
-    });
+    const idxSet = new Set(idxToDelete);
+    options.indexes = options.indexes.filter((_, idx) => !idxSet.has(idx));

---

898-899: Fix unreachable DOUBLE condition in attributeToSQL.

Line 898 compares attribute.type === 'DOUBLE' (strict equality with a string), but attribute.type is a DataType class instance. This condition is unreachable dead code and inconsistent with other branches that use instanceof (lines 870, 882, 889).

🔎 Apply this diff to fix:

-  } else if (attribute.type && attribute.type === 'DOUBLE') {
-    template = attribute.type.toSql();
+  } else if (attribute.type instanceof DataTypes.DOUBLE) {
+    template = attribute.type.toSql();

---

690-696: Fix auto-increment detection to check all rows, not just the last one.

The loop overwrites allColumns[key] for each row (lines 692-695), so only the last row's value determines the final state. A mix of null and non-null values for an auto-increment column is only detected when the last row is null. If the last row is non-null, the mix passes silently.

🔎 Apply this diff to enforce the invariant across all rows:

+    const sawNull = {};
+    const sawNonNull = {};
+
     for (const fieldValueHash of fieldValueHashes) {
       forOwn(fieldValueHash, (value, key) => {
-        allColumns[key] =
-          fieldMappedAttributes[key] &&
-          fieldMappedAttributes[key].autoIncrement === true &&
-          value === null;
+        if (!fieldMappedAttributes[key]?.autoIncrement) {
+          allColumns[key] = false;
+          return;
+        }
+
+        const isNull = value === null;
+        sawNull[key] ||= isNull;
+        sawNonNull[key] ||= !isNull;
+
+        if (sawNull[key] && sawNonNull[key]) {
+          throw new Error(
+            'For an auto-increment column either all rows must be null or non-null; a mix is not allowed.',
+          );
+        }
+
+        allColumns[key] = sawNull[key];
       });
     }

packages/oracle/src/dialect.ts (3)

109-111: getDefaultSchema will throw in non-replication setups.

Line 110 accesses this.sequelize.options.replication.write.username without safe optional chaining on replication or write. In default (non-replication) configurations, replication or write will be undefined, causing a runtime error before the optional chaining on username is evaluated.

🔎 Apply this diff to fix:

  getDefaultSchema(): string {
-   return this.sequelize.options.replication.write.username?.toUpperCase() ?? '';
+   const replicationUser = this.sequelize.options.replication?.write?.username;
+   const directUser = this.sequelize.options.username;
+   const username = replicationUser ?? directUser;
+   return username?.toUpperCase() ?? '';
  }

---

88-90: Update documentation URL to match minimum database version.

Line 88 links to Oracle Database 23 documentation, but line 89 declares minimumDatabaseVersion: '18.0.0'. Oracle 23 introduced features unavailable in 18 (native JSON SQL type, SQL Domains, spatial enhancements). The documentation should align with the declared minimum version.

🔎 Apply this diff:

-    dataTypesDocumentationUrl:
-      'https://docs.oracle.com/en/database/oracle/oracle-database/23/sqlrf/Data-Types.html',
+    dataTypesDocumentationUrl:
+      'https://docs.oracle.com/en/database/oracle/oracle-database/18/sqlrf/Data-Types.html',

---

141-146: Fix typos in comments.

Line 141: "indicated y the last empty string" should be "indicated by the last empty string"

Line 146: Comment mentions "TIMESTAM_TZ" but should be "TIMESTAMP_TZ"

🔎 Apply this diff:

     // Validate that the split result has exactly three parts (function name, parameters, and an empty string)
-    // and that there are no additional SQL commands after the function call (indicated y the last empty string).
+    // and that there are no additional SQL commands after the function call (indicated by the last empty string).
     if (splitVal.length !== 3 || splitVal[2] !== '') {
       throw new Error('Invalid SQL function call.'); // Error if function call has unexpected format
     }

-    // Extract the function name (either 'TO_TIMESTAM_TZ' or 'TO_DATE') and the contents inside the parentheses
+    // Extract the function name (either 'TO_TIMESTAMP_TZ' or 'TO_DATE') and the contents inside the parentheses