upsertedId & uspertedCount in UpdateResult could be confusing
richardscarrott opened this issue · 2 comments
I've always read "upsert" to mean "update or insert" (which could be wrong of course) however UpdateResult
implies it means "insert" given that UpdateResult.upsertedCount
and UpdateResult.upsertedId
are only set if an insert occurred.
I wonder if it'd be clearer if it was UpdateResult.insertedCount
and UpdateResult.insertedId
which would align more with
$setOnInsert
(i.e. it's not $setOnUpsert
)?
Perhaps a minor grievance but I always find myself having to double check + test how upsertedCount
behaves.
/**
* The number of documents that were upserted.
*
* NOT REQUIRED: Drivers may choose to not provide this property so long as
* it is always possible to infer whether an upsert has taken place. Since
* the "_id" of an upserted document could be null, a null "upsertedId" may
* be ambiguous in some drivers. If so, this field can be used to indicate
* whether an upsert has taken place.
*/
upsertedCount: Int64;
/**
* The identifier of the inserted document if an upsert took place.
*/
upsertedId: any;
"Upsert" is indeed a portmanteau of "update" and "insert" and predates MongoDB. AFAIK, driver documentation is consistent about always referring to an insert occurring through an update
command (i.e. with upsert: true
) as an "upsert".
I don't have an explanation for how the name for $setOnInsert
was chosen by the query team, but I can think of at least two justifications for the current field names in drivers:
- The server response for
update
commands includes anupserted
array, which is used to derive the driver result fields. upsertedCount
is also used in BulkWriteResult, which already has aninsertedCount
field that is specific to documents inserted viainsert
commands. This also extends to theupsertedIds
andinsertedIds
maps in BulkWriteResult. Inconsistent naming between UpdateResult and BulkWriteResult would likely invite more confusion than the situation you're describing.
I don't have an explanation for how the name for $setOnInsert was chosen by the query team ...
$setOnInsert
was introduced via SERVER-340 and seems to have considered a number of possible alternate names ($setDef
, $onInsert
, $setIfAbsent
). As a new document is being inserted (regardless of whether it is via an insert
or update+upsert
) it is a clearer directive than "setOnUpsert".
Closing this issue out as it doesn't appear any changes are being suggested, however this issue can be reopened later if needed.