Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(NODE-3695)!: remove lastop and optime from bulk result #3504

Merged
merged 5 commits into from
Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions etc/notes/CHANGES_5.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ The following is a detailed collection of the changes in the major v5 release of

## Changes

### Bulk results no longer contain `lastOp()` and `opTime`

The `lastOp()` method and `opTime` property on the `BulkResult` have been removed. Merging of bulk results
no longer normalizes the values. There is no new method or property to replace them.

### `CursorCloseOptions` removed

When calling `close()` on a `Cursor`, no more options can be provided. This removes support for the
Expand Down
63 changes: 1 addition & 62 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import {
BSONSerializeOptions,
Document,
Long,
ObjectId,
resolveBSONOptions,
Timestamp
} from '../bson';
import { BSONSerializeOptions, Document, ObjectId, resolveBSONOptions } from '../bson';
import type { Collection } from '../collection';
import {
AnyError,
Expand Down Expand Up @@ -146,7 +139,6 @@ export interface BulkResult {
nModified: number;
nRemoved: number;
upserted: Document[];
opTime?: Document;
}

/**
Expand Down Expand Up @@ -300,15 +292,6 @@ export class BulkWriteResult {
return this.result.writeErrors;
}

/**
* Retrieve lastOp if available
*
* @deprecated Will be removed in 5.0
*/
getLastOp(): Document | undefined {
return this.result.opTime;
}

/** Retrieve the write concern error if one exists */
getWriteConcernError(): WriteConcernError | undefined {
if (this.result.writeConcernErrors.length === 0) {
Expand Down Expand Up @@ -449,12 +432,6 @@ export class WriteError {
}
}

/** Converts the number to a Long or returns it. */
function longOrConvert(value: number | Long | Timestamp): Long | Timestamp {
// TODO(NODE-2674): Preserve int64 sent from MongoDB
return typeof value === 'number' ? Long.fromNumber(value) : value;
}

/** Merges results into shared data structure */
export function mergeBatchResults(
batch: Batch,
Expand Down Expand Up @@ -491,44 +468,6 @@ export function mergeBatchResults(
return;
}

// The server write command specification states that lastOp is an optional
// mongod only field that has a type of timestamp. Across various scarce specs
// where opTime is mentioned, it is an "opaque" object that can have a "ts" and
// "t" field with Timestamp and Long as their types respectively.
// The "lastOp" field of the bulk write result is never mentioned in the driver
// specifications or the bulk write spec, so we should probably just keep its
// value consistent since it seems to vary.
// See: https://github.com/mongodb/specifications/blob/master/source/driver-bulk-update.rst#results-object
if (result.opTime || result.lastOp) {
let opTime = result.lastOp || result.opTime;

// If the opTime is a Timestamp, convert it to a consistent format to be
// able to compare easily. Converting to the object from a timestamp is
// much more straightforward than the other direction.
if (opTime._bsontype === 'Timestamp') {
opTime = { ts: opTime, t: Long.ZERO };
}

// If there's no lastOp, just set it.
if (!bulkResult.opTime) {
bulkResult.opTime = opTime;
} else {
// First compare the ts values and set if the opTimeTS value is greater.
const lastOpTS = longOrConvert(bulkResult.opTime.ts);
const opTimeTS = longOrConvert(opTime.ts);
if (opTimeTS.greaterThan(lastOpTS)) {
bulkResult.opTime = opTime;
} else if (opTimeTS.equals(lastOpTS)) {
// If the ts values are equal, then compare using the t values.
const lastOpT = longOrConvert(bulkResult.opTime.t);
const opTimeT = longOrConvert(opTime.t);
if (opTimeT.greaterThan(lastOpT)) {
bulkResult.opTime = opTime;
}
}
}
}

// If we have an insert Batch type
if (isInsertBatch(batch) && result.n) {
bulkResult.nInserted = bulkResult.nInserted + result.n;
Expand Down
132 changes: 0 additions & 132 deletions test/unit/bulk/common.test.js

This file was deleted.