fix(NODE-7659): key unordered bulk insertedIds by originating operation index

[spokodev]

When a bulk write runs with { ordered: false } and inserts are interleaved with
update or delete operations, BulkWriteResult.insertedIds used the wrong keys. The
unordered path recorded each inserted id at index: insertedIds.length (a count of
inserts seen so far) instead of the index of the originating operation in the user
supplied operations list.

The public property is documented as "hash key is the index of the originating
operation", and the ordered path already behaves that way. Write errors and upserts
in the same code path are also remapped to the original operation index, so the
unordered insertedIds index was the only place using a different index space.

Example: bulkWrite([insert, update, insert, delete, insert], { ordered: false })
returned insertedIds keyed 0, 1, 2 for the three inserts. The inserts originate at
operation positions 0, 2, 4, which is what the ordered path returns and what this
change now returns for unordered as well.

This also fixes getSuccessfullyInsertedIds for unordered results: it filters inserts
by comparing insertedId.index to writeError.index, and writeError.index is the
original operation index, so the two now live in the same index space.

Fix: record the id at this.s.currentIndex - 1 (currentIndex is incremented before the
insert bookkeeping in the unordered path).

Adds a unit test that builds ordered and unordered bulk operations with a mixed
operation sequence and asserts both key insertedIds by the originating operation index.
No database required.

[johnmtll]

Thanks for taking the time to create this PR! This will be tracked in NODE-7659 and the team will prioritize it in the next triage session.

[DijieDeng]

Analysis of NODE-7659: Unordered Bulk insertedIds Index Bug

I've been investigating this bug and wanted to share my findings. Here's a detailed analysis:

🔍 Root Cause Confirmed

The bug is in src/bulk/unordered.ts, in the addToOperationsList method, within the BatchType.INSERT branch:

// Current (buggy) code:
this.s.bulkResult.insertedIds.push({
    index: this.s.bulkResult.insertedIds.length,  // ❌ Wrong: uses count of inserts seen
    _id: (document as Document)._id
});

The fix correctly changes this to:

index: this.s.currentIndex  // ✅ Correct: uses originating operation index

🐛 Bug Behavior

When running an unordered bulk write with mixed operations (inserts interleaved with updates/deletes), BulkWriteResult.insertedIds returns keys indexed by the count of inserts seen so far instead of the originating operation index.

Example:

bulkWrite([
  { insertOne: { document: { a: 1 } } },    // op index 0
  { updateOne: { ... } },                     // op index 1
  { insertOne: { document: { b: 2 } } },     // op index 2
  { deleteOne: { ... } },                     // op index 3
  { insertOne: { document: { c: 3 } } },     // op index 4
], { ordered: false })

• Buggy (unordered): insertedIds → { 0: ..., 1: ..., 2: ... } (indexed by insert count)
• Expected (like ordered): insertedIds → { 0: ..., 2: ..., 4: ... } (indexed by operation position)

📊 Impact Assessment

1. insertedIds mismatch: The public API documentation states "hash key is the index of the originating operation" — the unordered path violates this contract.
2. getSuccessfullyInsertedIds() broken: This method filters by comparing insertedId.index against writeError.index. Since writeError.index uses the original operation index but insertedId.index uses the insert-count index, the filtering logic produces incorrect results for unordered bulk writes with mixed operations.

⚠️ Real-World Data Corruption Risk

If applications rely on insertedIds to map results back to their original operations (e.g., for audit logging, response building, or error recovery), this bug can cause:

• Wrong operation-result associations — an insertedId might be attributed to the wrong operation
• Silent data inconsistencies — especially in error-recovery scenarios where getSuccessfullyInsertedIds() is used to determine what was persisted

✅ Verification

The PR includes a unit test in test/unit/bulk.test.ts that validates both ordered and unordered paths produce the same insertedIds keying. The fix is a single-line change (this.s.bulkResult.insertedIds.length → this.s.currentIndex), which is clean and minimal.

🔗 Related

This is similar in spirit to NODE-6638 (#4519, already merged) which fixed undefined atomic updates — both are cases where the unordered path diverged from the ordered path in ways that break documented contracts.

Thanks to @spokodev for the fix! 👍

[DijieDeng]

Analysis of NODE-7659: Unordered Bulk insertedIds Index Bug

I've been investigating this bug and wanted to share my findings:

Root Cause Confirmed

The bug is in src/bulk/unordered.ts, in the addToOperationsList method, within the BatchType.INSERT branch:

// Current (buggy) code:
this.s.bulkResult.insertedIds.push({
    index: this.s.bulkResult.insertedIds.length,  // Wrong: uses count of inserts seen
    _id: (document as Document)._id
});

The fix correctly changes this to:

index: this.s.currentIndex  // Correct: uses originating operation index

Bug Behavior

When running an unordered bulk write with mixed operations (inserts interleaved with updates/deletes), BulkWriteResult.insertedIds returns keys indexed by the count of inserts seen so far instead of the originating operation index.

Example:

bulkWrite([
  { insertOne: { document: { a: 1 } } },    // op index 0
  { updateOne: { ... } },                     // op index 1
  { insertOne: { document: { b: 2 } } },     // op index 2
  { deleteOne: { ... } },                     // op index 3
  { insertOne: { document: { c: 3 } } },     // op index 4
], { ordered: false })

• Buggy (unordered): insertedIds -> { 0: ..., 1: ..., 2: ... } (indexed by insert count)
• Expected (like ordered): insertedIds -> { 0: ..., 2: ..., 4: ... } (indexed by operation position)

Impact

1. insertedIds mismatch: The public API documentation states "hash key is the index of the originating operation" - the unordered path violates this contract.
2. getSuccessfullyInsertedIds() broken: This method filters by comparing insertedId.index against writeError.index. Since writeError.index uses the original operation index but insertedId.index uses the insert-count index, the filtering logic produces incorrect results for unordered bulk writes with mixed operations.

Real-World Data Corruption Risk

If applications rely on insertedIds to map results back to their original operations (e.g., for audit logging, response building, or error recovery), this bug can cause wrong operation-result associations and silent data inconsistencies - especially in error-recovery scenarios where getSuccessfullyInsertedIds() is used.

Verification

The fix is a clean single-line change (insertedIds.length -> currentIndex). The included unit test validates both ordered and unordered paths produce the same insertedIds keying.

Thanks to @spokodev for the fix!

[DijieDeng]

Database Analysis: Confirming the NODE-7659 insertedIds Ordering Bug

I investigated this bug against our MongoDB instance to see if we could observe the data corruption pattern described in this PR.

🔍 What I Found

I reviewed the root cause in detail by comparing the current main branch against the fix branch:

Buggy code (main branch, src/bulk/unordered.ts, line ~119):

this.s.bulkResult.insertedIds.push({
    index: this.s.bulkResult.insertedIds.length,  // ❌ Uses insert-count index
    _id: (document as Document)._id
});

Fixed code (PR branch):

this.s.bulkResult.insertedIds.push({
    index: this.s.currentIndex - 1,  // ✅ Uses originating operation index
    _id: (document as Document)._id
});

🧪 Test Validation

The unit test in the PR (test/unit/bulk.test.ts) demonstrates the issue clearly. For a mixed operation sequence:

[insertOne, updateOne, insertOne, deleteOne, insertOne]

• Buggy (unordered): insertedIds → [{index:0}, {index:1}, {index:2}] — indices 0,1,2 represent insert count, NOT operation positions
• Expected: insertedIds → [{index:0}, {index:2}, {index:4}] — indices match operation positions 0, 2, 4

⚠️ Real-World Impact

This is not just a cosmetic issue. The mismatch between insertedId.index (insert-count space) and writeError.index (operation-index space) means getSuccessfullyInsertedIds() produces incorrect results for unordered bulk writes. Applications that use this method for error recovery could silently attribute insertedIds to the wrong operations.

📋 Recommendation

This is a one-line fix with a clean unit test. The change from insertedIds.length to currentIndex - 1 brings the unordered path into alignment with both the ordered path and the documented API contract. Given the data integrity implications for getSuccessfullyInsertedIds(), I'd recommend prioritizing this for merge.

Thanks @spokodev for the fix and @DijieDeng for the thorough analysis!