-
Notifications
You must be signed in to change notification settings - Fork 895
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
GODRIVER-3285 Allow update to supply sort option. #1797
Conversation
API Change Report./v2/mongocompatible changes(*ReplaceOneModel).SetSort: added ./v2/mongo/optionscompatible changes(*ReplaceOptionsBuilder).SetSort: added |
fab1a09
to
8533d0a
Compare
8533d0a
to
5a87ba9
Compare
mongo/bulk_write_models.go
Outdated
// matched by the sort order will be replaced. This option is only valid for MongoDB versions >= 8.0. The driver will | ||
// return an error if the sort parameter is a multi-key map. The default value is nil. | ||
func (rom *ReplaceOneModel) SetSort(sort interface{}) *ReplaceOneModel { | ||
rom.Sort = &sort |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is taking the address of sort
intentional? If so, what is the purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fat-fingers... 😅
case "sort": | ||
sort, err := createSort(val) | ||
if err != nil { | ||
return nil, fmt.Errorf("error creating sort: %w", err) | ||
} | ||
opts.SetSort(sort) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The additional createSort
logic seems unnecessary. Can we do something similar to the "comment" case and just pass val
to SetSort
?
E.g.
opts.SetSort(val)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
GODRIVER-3285
Summary
Allow v2 update to supply sort option.
Sort option for the client bulk write will be added after #1884.
Background & Motivation