[DRIVERS-2725] BulkWriteResult.insertedIds does not reflect actual inserted documents Created: 15/Sep/23  Updated: 19/Sep/23

Status: Backlog
Project: Drivers
Component/s: CRUD
Fix Version/s: None

Type: Task Priority: Unknown
Reporter: Aditi Khare Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: leads-triage
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to NODE-5421 BulkWriteResult.insertedIds does not ... Closed
is related to DRIVERS-716 Improved Bulk Write API Designing
Driver Changes: Needed

 Description   

Summary

Currently, the drivers specification for BulkWriteResult provides an optional property `insertedIds` (see CRUD spec, MongoDB manual).

A user on Version 5.6.0 of the Node Driver reported the following bug on BulkWriteResult.insertedIds: If there are writeErrors and an _id is not correctly inserted, it will still incorrectly appear in the insertedIds Object.

This bug likely reproduce on other drivers variations that support BulkWriteResult.insertedIds.

Motivation

Who is the affected end user?

The affected end user is any user using BulkWrite or InsertMany with inserts that contain any errors.

How does this affect the end user?

In the event of an error case, the user will be confused as to which _ids were inserted. This is because BulkWriteResult.writeErrors will signal that certain inserts were unsuccessful, but BulkWriteResult.insertedIds will include all attempted inserts.

This can lead the user to be misinformed on which inserts were successful.

How likely is it that this problem or use case will occur?

Edge Case

If the problem does occur, what are the consequences and how severe are they?

Misinformation, potentially minor

Is this issue urgent?

N/A

Is this ticket required by a downstream team?

May affect Mongosh

Is this ticket only for tests?

No

Acceptance Criteria

  • BulkWrite and InsertMany support insertedIds correctly, in that insertedIds only includes index and _id pairs that were actually inserted
  • Tests cover unordered and ordered inserts, as well as inserts with multiple errors


 Comments   
Comment by Jeremy Mikola [ 18/Sep/23 ]

I believe this exact behavior was previously discussed in SPEC-82, where the outcome was to allow drivers to report the auto-generated ID even in the event of a write error.

The current wording of the CRUD spec does not address how to populate or interpret insertedIds when dealing with write errors, so the first step would be making a spec change to clarify that one way or another.

If we keep the existing behavior, the spec could be improved to mention that insertedIds may contain IDs of failed inserts and that the map should be interpreted alongside writeErrors to infer exactly which IDs were successfully inserted.

Whether we keep the existing behavior or change it (to prune insertedIds of errored inserts), we can consider adding test coverage.

This could be tested using expectedError.expectResult in the Unified Test Format to assert that erroneous inserts (e.g. duplicate key violations) are pruned from the insertedIds map on the BulkWriteResult exposed on a BulkWriteError. An additional test could also be created for insertMany operations, since those also throw BulkWriteError exceptions.

Since the insertedIds map is optional, these tests should use $$unsetOrMatches for their assertions (as is done in existing non-error tests for insertedIds).

Generated at Thu Feb 08 08:26:17 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.