Skip to content

GODRIVER-3370 Add bypassEmptyTsReplacement option. #1927

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

Merged
merged 4 commits into from
Feb 7, 2025

Conversation

qingyang-hu
Copy link
Collaborator

GODRIVER-3370

Summary

Add bypassEmptyTsReplacement option to:

  • insertOne
  • insertMany
  • updateOne
  • updateMany
  • replaceOne
  • findOneAndUpdate
  • findOneAndReplace
  • collection.bulkWrite

Background & Motivation

Correlated server ticket: SERVER-88750

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label Jan 31, 2025
Copy link
Contributor

mongodb-drivers-pr-bot bot commented Jan 31, 2025

API Change Report

./mongo/options

compatible changes

BulkWriteOptions.BypassEmptyTsReplacement: added
FindOneAndReplaceOptions.BypassEmptyTsReplacement: added
FindOneAndUpdateOptions.BypassEmptyTsReplacement: added
InsertManyOptions.BypassEmptyTsReplacement: added
InsertOneOptions.BypassEmptyTsReplacement: added
ReplaceOptions.BypassEmptyTsReplacement: added
UpdateOptions.BypassEmptyTsReplacement: added

./x/mongo/driver/operation

compatible changes

(*FindAndModify).BypassEmptyTsReplacement: added
(*Insert).BypassEmptyTsReplacement: added
(*Update).BypassEmptyTsReplacement: added

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@qingyang-hu qingyang-hu marked this pull request as ready for review February 3, 2025 16:26
@qingyang-hu qingyang-hu added priority-2-medium Medium Priority PR for Review and removed priority-3-low Low Priority PR for Review labels Feb 3, 2025
@@ -746,6 +752,12 @@ func (f *FindOneAndReplaceOptions) SetLet(let interface{}) *FindOneAndReplaceOpt
return f
}

// SetBypassEmptyTsReplacement sets the value for the BypassEmptyTsReplacement field.
func (f *FindOneAndReplaceOptions) SetBypassEmptyTsReplacement(b bool) *FindOneAndReplaceOptions {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be marked as deprecated with the same deprecation note as the corresponding field.

@@ -65,6 +71,12 @@ func (b *BulkWriteOptions) SetLet(let interface{}) *BulkWriteOptions {
return b
}

// SetBypassEmptyTsReplacement sets the value for the BypassEmptyTsReplacement field.
func (b *BulkWriteOptions) SetBypassEmptyTsReplacement(bypassEmptyTsReplacement bool) *BulkWriteOptions {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the public setters should be marked as deprecated with the same deprecation note as the corresponding field.

Also applies to SetBypassEmptyTsReplacement on:

  • FindOneAndReplaceOptions
  • InsertOneOptions
  • ReplaceOptions
  • UpdateOptions

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After re-reading the existing code, I noticed the Go driver doesn't provide setters for internal options. Going to remove these methods.

@prestonvasquez prestonvasquez added priority-1-high High Priority PR for Review and removed priority-2-medium Medium Priority PR for Review labels Feb 6, 2025
matthewdale
matthewdale previously approved these changes Feb 6, 2025
Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

prestonvasquez
prestonvasquez previously approved these changes Feb 6, 2025
Copy link
Member

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some minor test updates

t.Helper()

valType, data, err := bson.MarshalValue(val)
assert.Nil(t, err, "MarshalValue error: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.Nil(t, err, "MarshalValue error: %v", err)
require.NoError(t, err, "MarshalValue error: %v", err)

Suggest stopping test execution when we get an unexpected error.

require.NoError(mt, err, "InsertOne error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

Suggest not stoping the test if we get an unexpected result we are testing for.

require.NoError(mt, err, "InsertMany error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

Suggest not stoping the test if we get an unexpected result we are testing for.

require.NoError(mt, err, "UpdateOne error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

Suggest not stoping the test if we get an unexpected result we are testing for.

require.NoError(mt, err, "UpdateMany error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

Suggest not stoping the test if we get an unexpected result we are testing for.

require.NoError(mt, err, "ReplaceOne error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

require.NoError(mt, err, "FindOneAndUpdate error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

require.NoError(mt, err, "FindOneAndReplace error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

require.NoError(mt, err, "BulkWrite error: %v", err)
evt := mt.GetStartedEvent()
val := evt.Command.Lookup("bypassEmptyTsReplacement")
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())
assert.Equal(mt, tc.expected, val, "expected bypassEmptyTsReplacement to be %s", tc.expected.String())

@qingyang-hu qingyang-hu merged commit e796c82 into mongodb:v1 Feb 7, 2025
30 of 34 checks passed
@matthewdale
Copy link
Collaborator

drivers-pr-bot please backport to release/1.17

mongodb-drivers-pr-bot bot pushed a commit that referenced this pull request Feb 20, 2025
matthewdale pushed a commit that referenced this pull request Feb 20, 2025
….17] (#1954)

Co-authored-by: Qingyang Hu <103950869+qingyang-hu@users.noreply.github.com>
alcaeus added a commit that referenced this pull request Mar 4, 2025
* release/1.17:
  Use different credentials for merge-up PRs (#1968)
  Add GitHub Actions workflow for merge ups (#1962)
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  BUMP v1.17.1
  GODRIVER-3156 Detect and discard closed idle connections. (#1815) [release/1.17] (#1841)
  GODRIVER-3313 [release/1.17] Skip CSOT spec tests on Windows and macOS. (#1838)
  GODRIVER-3358 [release/1.17] Do not override authSource from TXT record (#1840)
  GODRIVER-2589 [release/1.17] Clarify `*Cursor.All()` behavior in comment. (#1839)
  DEVPROD-10453 Use assume_role for s3 uploads [release/1.17] (#1824) (#1837)
  update repo metadata
alcaeus added a commit that referenced this pull request Mar 4, 2025
* commit '9c485751':
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  BUMP v1.17.1
  GODRIVER-3156 Detect and discard closed idle connections. (#1815) [release/1.17] (#1841)
  GODRIVER-3313 [release/1.17] Skip CSOT spec tests on Windows and macOS. (#1838)
  GODRIVER-3358 [release/1.17] Do not override authSource from TXT record (#1840)
  GODRIVER-2589 [release/1.17] Clarify `*Cursor.All()` behavior in comment. (#1839)
  DEVPROD-10453 Use assume_role for s3 uploads [release/1.17] (#1824) (#1837)
  update repo metadata
alcaeus added a commit that referenced this pull request Mar 4, 2025
* commit '0dc2e05e': (184 commits)
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927)
  BUMP v1.17.2
  GODRIVER-3436 Avoid initializing null data given custom decoder (#1902)
  GODRIVER-3340 Add a test for goroutine leaks. (#1874)
  Update reviewers.txt (#1855) [v1] (#1883)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [v1] (#1869)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  GODRIVER-3156 Detect and discard closed idle connections. (#1815)
  GODRIVER-3358 Do not override authSource from TXT record (#1830)
  DEVPROD-10453 Use assume_role for s3 uploads [v1] (#1824)
  GODRIVER-2589 Clarify `*Cursor.All()` behavior in comment. (#1804)
  GODRIVER-3313 Skip CSOT spec tests on Windows and macOS. (#1818)
  BUMP v1.17.0
  GODRIVER-3302 Handle malformatted message length properly. (#1758)
  GODRIVER-3312 Use remaining test secrets from the vault [v1] (#1811)
  Remove GCP from supplied callback example (#1809)
  ...
alcaeus added a commit that referenced this pull request Mar 4, 2025
* v1:
  Use different credentials for merge-up PRs (#1968)
  Add GitHub Actions workflow for merge ups (#1962)
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  BUMP v1.17.1
  GODRIVER-3156 Detect and discard closed idle connections. (#1815) [release/1.17] (#1841)
  GODRIVER-3313 [release/1.17] Skip CSOT spec tests on Windows and macOS. (#1838)
  GODRIVER-3358 [release/1.17] Do not override authSource from TXT record (#1840)
  GODRIVER-2589 [release/1.17] Clarify `*Cursor.All()` behavior in comment. (#1839)
  DEVPROD-10453 Use assume_role for s3 uploads [release/1.17] (#1824) (#1837)
  update repo metadata
alcaeus added a commit that referenced this pull request Mar 4, 2025
…41094483929

* release/2.0: (203 commits)
  Use different credentials for merge-up PRs (#1968)
  Add GitHub Actions workflow for merge ups (#1962)
  BUMP v1.17.3
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943) [release/1.17] (#1957)
  Update reviewers.txt (#1855) [v1] (#1883) [release/1.17] (#1958)
  Cherry pick 1.17.2 work to release/1.17 (#1956)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924) (#1955)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927) [release/1.17] (#1954)
  GODRIVER-3448 Limit GOMAXPROCS for fuzz tests (#1939) [v1] (#1943)
  GODRIVER-3470 Correct BSON unmarshaling logic for null values (#1924)
  GODRIVER-3370 Add bypassEmptyTsReplacement option. (#1927)
  BUMP v1.17.2
  GODRIVER-3436 Avoid initializing null data given custom decoder (#1902)
  GODRIVER-3340 Add a test for goroutine leaks. (#1874)
  Update reviewers.txt (#1855) [v1] (#1883)
  GODRIVER-3340 Bump github.com/klauspost/compress from 1.13.6 to 1.16.7 [release/1.17] (#1880)
  GODRIVER-3374 Add ReadCompressedCompressedMessage back to wiremessage API (#1870)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  Bump golangci-lint for 1.23 compatibility [v1] (#1875)
  Fix data race in 'discard connections' pool test. [v1] (#1877)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-1-high High Priority PR for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants