feat(NODE-4877): Add support for useBigInt64 #3519

W-A-James · 2023-01-11T15:23:41Z

Description

Add support for BSON's useBigInt64 flag to the driver

What is changing?

useBigInt64 flag is now a part of the BSONSerializationOptions interface
useBigInt64 flag is now therefore part of all implementers of this interface

Is there new documentation needed for these changes?

No

What is the motivation for this change?

Integrating the BigInt64 support in the BSON library into the driver

Double check the following

Ran npm run check:lint script
Self-review completed using the steps outlined here
PR title follows the correct format: type(NODE-xxxx)[!]: description
- Example: feat(NODE-1234)!: rewriting everything in coffeescript
Changes are covered by tests
New TODOs have a related JIRA ticket
- NODE-5072 Change pattern for setting BSON options
- NODE-5073 Remove unused applyCommonQueryOptions function

W-A-James · 2023-01-17T21:58:25Z

Link to evergreen patch with useBigInt64=true by default to
see what, if anything, breaks. https://spruce.mongodb.com/version/63c7199232f4171734d3a529/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

nbbeeken · 2023-01-18T14:45:58Z

I think the option isn't actually making it to the BSON layer, we "sanitize" the options at the wire protocol layer to make sure that arbitrary options don't reach the bson deserializer, take a look at #2840

dariakp · 2023-01-24T21:17:40Z

This PR cannot merge until we do the 5.0 release.

nbbeeken · 2023-01-24T21:22:51Z

Can you post an updated link to an evg patch where useBigInt64 is set to true anywhere we call BSON.deserialize in the driver (should be four places in commands.ts). We want to see if the errors from that are salient to what can occur when the option is enabled

some errors might just be assertions no longer being equal, that would be expected

W-A-James · 2023-01-24T21:26:51Z

Evergreen patch with useBigInt64=true as default:

https://spruce.mongodb.com/version/63d04caea4cf47516296a359/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

nbbeeken · 2023-01-25T15:22:56Z

Lot's of red, unfortunately enough to make it so we can't see the output, atlas connectivity failing though might be a good place to start as that "shouldn't" have been impacted at all.

// TODO(NODE-2674): Preserve int64 sent from MongoDB

You'll want to look for this comment, likely every spot needs an extra condition to accept bigint

test/integration/node-specific/bson-options/useBigInt64.test.ts

src/sdam/monitor.ts

src/sdam/server_description.ts

src/sessions.ts

test/integration/change-streams/change_stream.test.ts

durran · 2023-02-10T16:59:13Z

test/integration/node-specific/bson-options/promote_values.test.js

-    test: function (done) {
-      var configuration = this.configuration;
-      var client = configuration.newClient(configuration.writeConcernMax(), {
+    test: async function () {


Note for other reviewers these tests are unrelated to the work in the PR and are just changing from callbacks to async/await.

test/integration/crud/insert.test.js

test/integration/node-specific/bson-options/useBigInt64.test.ts

src/connection_string.ts

nbbeeken

LGTM can't wait to use the biggest of ints!! 🎉

…ptions.promoteValues

test/integration/node-specific/bson-options/useBigInt64.test.ts

src/connection_string.ts

test/integration/change-streams/change_stream.test.ts

src/cursor/abstract_cursor.ts

test/integration/node-specific/bson-options/useBigInt64.test.ts

baileympearson · 2023-02-22T15:34:22Z

test/integration/node-specific/bson-options/useBigInt64.test.ts

@@ -0,0 +1,234 @@
+import { expect } from 'chai';


this file should use snake case (use_bigint_64.test.ts) or something similar to follow our existing filenaming convention.

test/integration/node-specific/bson-options/useBigInt64.test.ts

test/tools/unified-spec-runner/match.ts

Co-authored-by: Bailey Pearson <[email protected]>

W-A-James force-pushed the NODE-4877/add-support-for-BSON-useBigInt64-flag branch from 7f8f32d to 0125330 Compare January 17, 2023 19:32

W-A-James marked this pull request as ready for review January 24, 2023 21:10

dariakp added the Blocked Blocked on other work label Jan 24, 2023

W-A-James added Blocked Blocked on other work and removed Blocked Blocked on other work labels Jan 24, 2023

W-A-James marked this pull request as draft January 25, 2023 21:41

feat(NODE-4950)!: remove bson-ext import and squash commits

d54383d

W-A-James force-pushed the NODE-4877/add-support-for-BSON-useBigInt64-flag branch from d330f3f to d54383d Compare February 8, 2023 20:42

Merge branch 'main' into NODE-4877/add-support-for-BSON-useBigInt64-flag

6eea5be

This comment was marked as outdated.

Sign in to view

W-A-James marked this pull request as ready for review February 8, 2023 21:00

nbbeeken removed the Blocked Blocked on other work label Feb 8, 2023

ci(NODE-4877): Fix generate_evergreen_tasks.js

9f623a7

durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Feb 10, 2023

durran requested changes Feb 10, 2023

View reviewed changes

W-A-James added the Blocked Blocked on other work label Feb 13, 2023

W-A-James added 3 commits February 15, 2023 11:16

test(NODE-4877): Work on resolving review comments

ddda8f8

test(NODE-4877): Change test name

6ac4651

test(NODE-4877): Add new test

330a27a

durran removed the Blocked Blocked on other work label Feb 16, 2023

W-A-James added 4 commits February 16, 2023 14:16

test(NODE-4877): add new tests

90a5cea

test(NODE-4877): Update tests

2e40c2b

fix(NODE-4877): Add validation for bson options

cde8f0a

fix(NODE-4877): revert unneeded change

bbf39e8

baileympearson requested changes Feb 21, 2023

View reviewed changes

src/connection_string.ts Outdated Show resolved Hide resolved

src/connection_string.ts Outdated Show resolved Hide resolved

W-A-James added 5 commits February 21, 2023 09:53

fix(NODE-4877): Revert unneeded change to unused function

c7ad82c

chore(NODE-4877): Bump bson version

f29d8f5

test(NODE-4877): Make test more idiomatic

e5b0430

fix(NODE-4877): Add bigint to mongo IntegerType and NestedPaths

5073ce3

test(NODE-4877): Remove unneeded NOTE comment

4f50116

W-A-James requested review from baileympearson and nbbeeken February 21, 2023 15:05

nbbeeken previously approved these changes Feb 21, 2023

View reviewed changes

test(NODE-4877): Add tests to check that BSONErrors get thrown

35c7a2c

W-A-James dismissed nbbeeken’s stale review via 35c7a2c February 21, 2023 19:04

fix(NODE-4877): Add check for existence of options.promoteLongs and o…

01447c7

…ptions.promoteValues

baileympearson requested changes Feb 21, 2023

View reviewed changes

test/integration/node-specific/bson-options/useBigInt64.test.ts Outdated Show resolved Hide resolved

src/connection_string.ts Outdated Show resolved Hide resolved

W-A-James added 3 commits February 21, 2023 15:52

fix(NODE-4877): fix condition

7e903a3

test(NODE-4877): remove setting promoteValues and promoteLongs

f178c82

test(NODE-4877): Remove explicit calls to connect

61e1370

W-A-James requested a review from baileympearson February 21, 2023 21:12

style(NODE-4877): eslint

efcf0b9

baileympearson requested changes Feb 22, 2023

View reviewed changes

W-A-James and others added 6 commits February 22, 2023 11:04

Update test/integration/change-streams/change_stream.test.ts

3485472

Co-authored-by: Bailey Pearson <[email protected]>

test(NODE-4877): test reorganization

3e3aef3

test(NODE-4877): Add operation-level option superceding tests

f33339f

test(NODE-4877): Add type annotations

c8c0b36

Merge branch 'main' into NODE-4877/add-support-for-BSON-useBigInt64-flag

e4dd63a

test(NODE-4877): remove unneeded import

4e3469d

baileympearson approved these changes Feb 23, 2023

View reviewed changes

W-A-James requested a review from durran February 23, 2023 14:31

durran approved these changes Feb 23, 2023

View reviewed changes

durran merged commit 917668c into main Feb 23, 2023

durran deleted the NODE-4877/add-support-for-BSON-useBigInt64-flag branch February 23, 2023 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(NODE-4877): Add support for useBigInt64 #3519

feat(NODE-4877): Add support for useBigInt64 #3519

W-A-James commented Jan 11, 2023 •

edited

Loading

W-A-James commented Jan 17, 2023

nbbeeken commented Jan 18, 2023

dariakp commented Jan 24, 2023

nbbeeken commented Jan 24, 2023 •

edited

Loading

W-A-James commented Jan 24, 2023

nbbeeken commented Jan 25, 2023

This comment was marked as outdated.

durran Feb 10, 2023

nbbeeken left a comment

baileympearson Feb 22, 2023

feat(NODE-4877): Add support for useBigInt64 #3519

feat(NODE-4877): Add support for useBigInt64 #3519

Conversation

W-A-James commented Jan 11, 2023 • edited Loading

Description

What is changing?

Is there new documentation needed for these changes?

What is the motivation for this change?

Double check the following

W-A-James commented Jan 17, 2023

nbbeeken commented Jan 18, 2023

dariakp commented Jan 24, 2023

nbbeeken commented Jan 24, 2023 • edited Loading

W-A-James commented Jan 24, 2023

nbbeeken commented Jan 25, 2023

This comment was marked as outdated.

durran Feb 10, 2023

Choose a reason for hiding this comment

nbbeeken left a comment

Choose a reason for hiding this comment

baileympearson Feb 22, 2023

Choose a reason for hiding this comment

W-A-James commented Jan 11, 2023 •

edited

Loading

nbbeeken commented Jan 24, 2023 •

edited

Loading