packages/core/test/unit/dialects/mysql/query.test.js (1)

39-45: Consider using Support.getTestDialectTeaser() for the test description.

According to the coding guidelines, dialect-specific tests should use Support.getTestDialectTeaser() in their descriptions. Since this test suite is specific to MySQL/MariaDB behavior, consider updating the describe block:

-  describe('formatResults', () => {
+  describe(Support.getTestDialectTeaser('MYSQL'), 'formatResults', () => {

However, since the parent describe block at line 12 already indicates [MYSQL/MARIADB Specific], this may be acceptable as-is.

As per coding guidelines

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/core/test/integration/query-interface/bulk-insert.test.ts (1)

33-33: Use Support.getTestDialectTeaser() for dialect-specific test descriptions.

The coding guidelines specify using Support.getTestDialectTeaser() for dialect-specific test descriptions. This helper provides consistent formatting and makes it clear when tests have dialect-specific behavior.

Based on coding guidelines.

Apply this diff:

-  it('should insert multiple rows and return inserted data according to dialect', async function testBulkInsert() {
+  it(`${Support.getTestDialectTeaser('should insert multiple rows and return inserted data')}`, async function testBulkInsert() {

packages/mariadb/src/query.js (3)

125-137: Add warning comment about sequential ID assumption.

The code assumes that IDs will be sequential (startId, startId+1, startId+2, etc.), similar to the model-context path (lines 107-123). However, the existing code includes an important warning (lines 111-112) that this only works when @auto_increment_increment is set to 1 and doesn't work with GALERA clusters. This same warning should be present for the non-model-context path.

Apply this diff to add the warning comment:

         // No model context, in queryinterface,bulkinsert for example
+        // ONLY TRUE IF @auto_increment_increment is set to 1 !!
+        // Doesn't work with GALERA => each node will reserve increment (x for first server, x+1 for next node...)
         const startId = Number(data[this.getInsertIdField()]);
         if (data.affectedRows > 1) {

---

126-126: Verify type consistency: Number() vs BigInt arithmetic.

Line 126 uses Number() to convert startId, while the model-context path (line 119) uses BigInt arithmetic (startId + i where i is BigInt). If the database uses large ID values exceeding Number.MAX_SAFE_INTEGER (2^53 - 1), this could cause precision loss.

For consistency and to handle large IDs safely, consider using BigInt here as well, especially since the model-context path already does so.

Apply this diff if BigInt consistency is desired:

-        const startId = Number(data[this.getInsertIdField()]);
+        const startId = BigInt(data[this.getInsertIdField()]);
         if (data.affectedRows > 1) {
           result = [];
-          for (let i = startId; i < startId + data.affectedRows; i++) {
+          for (let i = 0n; i < data.affectedRows; i++) {
-            result.push({ id: i });
+            result.push({ id: Number(startId + i) });
           }
 
           return [result, data.affectedRows];
         }
 
         // Fallback: single insert ID
-        return [startId, data.affectedRows];
+        return [Number(startId), data.affectedRows];

---

127-137: Consider return type consistency for single-row inserts.

When multiple rows are affected (lines 128-134), the function returns an array of objects like [{id: 1}, {id: 2}]. However, for single-row inserts (line 137), it returns a bare number directly: [startId, data.affectedRows].

This inconsistency means callers must handle two different shapes:

• Multi-row: [[{id: 1}, {id: 2}], 2]
• Single-row: [1, 1]

For better consistency, consider wrapping the single-row ID in an object as well: [{id: startId}, data.affectedRows]. This would make the return shape predictable regardless of the number of rows inserted.

Apply this diff if consistent return shape is desired:

         // Fallback: single insert ID
-        return [startId, data.affectedRows];
+        return [{ id: startId }, data.affectedRows];

Note: This change would need corresponding updates in callers and tests that expect the bare number for single-row inserts.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/core/test/integration/query-interface/bulk-insert.test.ts (1)

31-49: Add a test case for single-row insert to verify array return.

The multi-row test correctly validates the new behavior, but the PR context indicates that previously, inserting 2 rows returned a scalar 1 instead of [1, 2]. To prevent regression, add a test case that inserts a single row and asserts the result is still an array (e.g., [{ id: 1 }] for MySQL/MariaDB), not a scalar.

Add this test case after the existing test:

it('should return an array even when inserting a single row', async function testSingleRowInsert() {
  const result = await this.queryInterface.bulkInsert('UsersBulkInsert', [{ name: 'SingleUser' }], {
    returning: true,
  });

  if (['mysql', 'mariadb'].includes(dialect)) {
    expect(result).to.deep.equal([{ id: 1 }]);
  } else {
    expect(result).to.deep.equal([{ id: 1, name: 'SingleUser' }]);
  }
});

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
PR: sequelize/sequelize#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/core/test/integration/query-interface/bulk-insert.test.ts (1)

5-9: Consider using ES6 imports for consistency.

The coding guidelines state "Write all new implementations in TypeScript" for files in packages/**/src/**/*.js, but this is a test file (.test.ts). While require() works, ES6 imports would be more consistent with TypeScript best practices.

Consider applying this diff:

-const chai = require('chai');
-
-const expect = chai.expect;
-const Support = require('../support');
-const { DataTypes } = require('@sequelize/core');
+import chai from 'chai';
+import Support from '../support';
+import { DataTypes } from '@sequelize/core';
+
+const expect = chai.expect;

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
PR: sequelize/sequelize#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/core/test/unit/dialects/mysql/query.test.js (1)

46-64: LGTM!

The test correctly validates the formatResults behavior for bulk inserts with a model context. It properly verifies that an array of ID objects is generated from the insertId through affectedRows.

Consider adding tests for edge cases:

• affectedRows: 0 (no rows inserted)
• String insertId values (the implementation has special handling: typeof startId === 'string' ? i.toString() : Number(i))
• Very large insertId values where BigInt handling is relevant

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/mysql/src/query.js (1)

146-169: Implementation looks correct; consider reducing code duplication.

The no-model bulk insert logic correctly generates an array of { id } objects from startId to startId + affectedRows - 1, which addresses the core issue in #14325.

However, there are opportunities for improvement:

1. Redundant branches (lines 161-165): Same issue as the model case—both the isString and else branches perform identical conversions.

2. Code duplication: The type detection and conversion logic is duplicated between the model case (lines 119-144) and this no-model case. Consider extracting a helper function to reduce duplication.

3. Inconsistent increment style (line 156): Uses i++ instead of i = i + 1n as in line 128. While both are valid for BigInt, consistency improves readability.

Apply this diff to simplify and improve consistency:

-          const start = isBigInt ? startId : BigInt(startId);
-          const end = start + BigInt(data.affectedRows);
+          const start = isBigInt ? startId : BigInt(startId);
+          const end = start + BigInt(data.affectedRows);

           for (let i = start; i < end; i++) {
-            let value;
-            if (isBigInt) {
-              value = i;
-            } // keep BigInt intact
-            else if (isString) {
-              value = Number(i);
-            } else {
-              value = Number(i);
-            }
+            const value = isBigInt ? i : Number(i);

             result.push({ id: value });
           }

Optional: Extract a shared helper function to eliminate duplication between lines 119-144 and 146-169:

/**
 * Converts a BigInt loop variable to the appropriate type based on the original startId type.
 * @param {bigint} value - The BigInt value to convert
 * @param {boolean} isBigInt - Whether the original startId was a BigInt
 * @returns {bigint|number} The converted value
 */
function convertInsertId(value, isBigInt) {
  return isBigInt ? value : Number(value);
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/mysql/src/query.js (1)

121-140: Consider adding safeguards for edge cases.

The generateInsertResults function is well-structured, but there are a few edge cases to consider:

1. Large numbers and precision loss: When converting BigInt to Number (line 118), values larger than Number.MAX_SAFE_INTEGER (9007199254740991) will lose precision. While this is an expected JavaScript limitation, it could lead to incorrect IDs for very large sequences.

2. Negative or invalid affectedRows: If affectedRows is negative or NaN, the loop will not execute and return an empty array, which may not reflect the actual error state.

3. Performance: For very large affectedRows values (e.g., 100,000+), the loop could impact performance. Consider whether a warning or optimization is needed for bulk operations.

Consider adding input validation at the beginning of the function:

 const generateInsertResults = (startId, affectedRows, keyName) => {
+  if (affectedRows < 0 || !Number.isFinite(affectedRows)) {
+    throw new Error(`Invalid affectedRows value: ${affectedRows}`);
+  }
+
   const isBigInt = typeof startId === 'bigint';
   const start = isBigInt ? startId : BigInt(startId);
   const end = start + BigInt(affectedRows);

Alternatively, verify if upstream code (e.g., the database driver) guarantees affectedRows is always a valid non-negative integer.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/mysql/src/query.js (1)

110-160: Consider TypeScript migration for new implementations.

The coding guidelines state: "Write all new implementations in TypeScript; avoid creating new .js files in src." While this PR modifies an existing .js file rather than creating a new one, the new helper functions (convertInsertId and generateInsertResults) represent new implementations. Consider migrating this file to TypeScript in a future PR to align with the guidelines and benefit from type safety.

As per coding guidelines.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro