fix(core): ensure bulkInsert returns array of inserted IDs
Pull Request Checklist
- [x] Have you added new tests to prevent regressions?
- [ ] If a documentation update is necessary, have you opened a PR to the documentation repository?
- [x] Did you update the typescript typings accordingly (if applicable)?
- [x] Does the description below contain a link to an existing issue (Closes #14325)?
- [x] Does the name of your PR follow our conventions?
Description of Changes
This PR ensures queryInterface.bulkInsert consistently returns an array of inserted IDs across mysql dialect, instead of a single value or undefined.
- Updated
bulkInsertimplementation to return an array.
Previously:
await queryInterface.bulkInsert('companies', [
{ name: 'Example Company 1' },
{ name: 'Example Company 2' },
]);
// → 1
now:
await queryInterface.bulkInsert('companies', [
{ name: 'Example Company 1' },
{ name: 'Example Company 2' },
]);
// → [1, 2]
Closes #14325
List of Breaking Changes
- None (the return type is extended, but still backward-compatible).
Summary by CodeRabbit
-
New Features
- Bulk inserts now consistently return arrays of id objects for multi-row inserts in MySQL/MariaDB; single-row and empty inserts unchanged.
- Insert ID types (number, string, BigInt) are preserved and normalized across both model and no-model insert paths.
-
Tests
- Added integration tests validating bulkInsert return shapes across dialects and single-row behavior.
- Expanded unit tests for result formatting and related logging.
Walkthrough
Modify MySQL and MariaDB insert result formatting so bulk inserts return consistent arrays of { id }-style objects derived from the driver's insertId and affectedRows; add unit tests for MySQL formatResults and an integration test for QueryInterface.bulkInsert.
Changes
| Cohort / File(s) | Summary |
|---|---|
MySQL insert formattingpackages/mysql/src/query.js |
Add convertInsertId and generateInsertResults; replace per-iteration ID construction with range-based generation to return an array of { <pkName>: value } for bulk inserts (with or without model), preserving original id types (Number, String, BigInt). |
MariaDB insert formattingpackages/mariadb/src/query.js |
Change no-model bulk insert path to coerce startId from data[this.getInsertIdField()] and build an array [{ id: startId }, ...] for affectedRows, returning the array instead of a single insertId. |
Unit tests (MySQL)packages/core/test/unit/dialects/mysql/query.test.js |
Initialize query.sql in logWarnings test; add formatResults tests covering model-context bulk, no-model bulk, single-row, zero-rows, string insertId, and BigInt insertId, asserting returned arrays and preserved id types. |
Integration tests (QueryInterface.bulkInsert)packages/core/test/integration/query-interface/bulk-insert.test.ts |
New integration suite creating/dropping UsersBulkInsert; tests bulkInsert(..., { returning: true }) expectations across dialects (MySQL/MariaDB -> IDs-only array; others -> full rows) and verifies single-row returns array. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant App
participant QI as QueryInterface
participant Conn as Dialect Connection
participant Q as DialectQuery.formatResults
App->>QI: bulkInsert(table, rows, { returning: true })
QI->>Conn: Execute INSERT (bulk)
Conn-->>QI: ResultSetHeader { insertId, affectedRows, ... }
QI->>Q: formatResults(ResultSetHeader, options)
alt model context present
Q-->>QI: Array of inserted row objects (model-mapped/full rows)
else no model context
alt affectedRows > 1
Q-->>QI: Array of `{ id: startId + n }` for n = 0..affectedRows-1
else affectedRows == 1
Q-->>QI: Array with single `{ id: startId }`
else affectedRows == 0
Q-->>QI: Empty array
end
end
QI-->>App: Return dialect-specific result
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Thump — I counted startId in the moonlight glow,
Each hop became a little id arranged in row.
Numbers, strings, BigInts kept tidy and neat,
From one to many, every id found its seat.
Hooray — bulk inserts lined up, all complete! 🐇
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes Check | ⚠️ Warning | This PR extends changes to the MariaDB dialect’s formatResults implementation and alters unrelated unit tests for logWarnings and integration tests for non-MySQL dialects, which exceed the original scope of fixing MySQL bulkInsert return values in issue #14325. These modifications are not directly required to meet the linked issue’s objective and introduce additional surface area beyond the targeted dialect and functionality. Consolidating unrelated changes risks conflating separate concerns and complicates the review and testing of the core MySQL fix. | Separate the MariaDB dialect adjustments and unrelated logWarnings or non-MySQL integration test changes into distinct PRs or issues, focusing this PR solely on the MySQL bulkInsert return behavior to align with issue #14325. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title Check | ✅ Passed | The title succinctly and accurately summarizes the primary change, which is updating bulkInsert in core to return an array of inserted IDs. It follows the conventional “fix(scope): description” style and conveys the main intent clearly without extraneous detail. |
| Linked Issues Check | ✅ Passed | The PR modifies MySQL dialect’s formatResults to build and return an array of inserted ID objects for multi-row bulkInsert operations as specified in issue [#14325], and adds corresponding unit and integration tests to verify array returns across scenarios. Tests cover various insertId types and affectedRows values, demonstrating that bulkInsert now aligns with the expected behavior of returning an array of new record IDs. Therefore, the supplied changes satisfy the linked issue’s primary requirement. |
| Docstring Coverage | ✅ Passed | No functions found in the changes. Docstring coverage check skipped. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
Please adjust the tests and probably add new ones. It could be that we have tests that only check for certain dialects, you'll need to update those expectations as well to include mysql
Hi , it looks like the postgres oldest (Node 18) CI job failed due to a network issue while downloading dependencies (connection reset by peer). All other CI checks have passed . Could someone please retry this job when convenient? Thanks!
Hi @WikiRik I’ve addressed the feedback and updated the tests. CI checks are passed except one falls due to netwok issue. Could you please take another look when you have time? Thanks a lot for your guidance
Hi @WikiRik @sdepold , just checking in — is there anything blocking this PR or anything I should update? Thanks a lot for your time!