Skip to content

DOCSP-49975 Specify Documents to Return #503

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 5 commits into from
Jun 2, 2025

Conversation

lindseymoore
Copy link
Collaborator

@lindseymoore lindseymoore commented May 27, 2025

Pull Request Info

Moves info from relevant pages onto one page.

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-49975

Staging Links

https://deploy-preview-503--docs-golang.netlify.app/crud/query/specify-return-documents/

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?
  • Are the page titles greater than 20 characters long and SEO relevant?

Copy link

netlify bot commented May 27, 2025

Deploy Preview for docs-golang ready!

Name Link
🔨 Latest commit acbd03c
🔍 Latest deploy log https://app.netlify.com/projects/docs-golang/deploys/683e152fa17fe10008ae5e13
😎 Deploy Preview https://deploy-preview-503--docs-golang.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Collaborator

@mcmorisi mcmorisi left a comment

Choose a reason for hiding this comment

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

Just a few suggestions!

sort fields and direction to the ``SetSort()`` method of an operation's options.

The following operations take options as a parameter:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The following operations take options as a parameter:
The following operations take sort options as a parameter:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reworded!

the read operation's options.

The following read operations take an options object as a parameter:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The following read operations take an options object as a parameter:
The following read operations take skip options as a parameter:

Comment on lines 290 to 293

Passing in a negative number to the ``SetSkip()`` method results
in a runtime error.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is funny to imagine someone attempting to skip negative documents – do you feel this is worth specifically calling out or can we trim this out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed!

Comment on lines 474 to 478
opts := options.Find().SetLimit(2).SetSort(bson.D{{"enrollment", -1}}).SetSkip(1)
opts := options.Find().SetLimit(2).SetSkip(1).SetSort(bson.D{{"enrollment", -1}})
opts := options.Find().SetSkip(1).SetSort(bson.D{{"enrollment", -1}}).SetLimit(2)
opts := options.Find().SetSkip(1).SetLimit(2).SetSort(bson.D{{"enrollment", -1}})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure I'm a fan of having the code block inside the admonition here – could we either take this out of the admonition or reword this to say "Performing the limit, sort, and skip options in any order will produce the same result"?

Copy link
Collaborator Author

@lindseymoore lindseymoore May 30, 2025

Choose a reason for hiding this comment

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

Edited and moved the example to the top of the section! Order matters for sort and skip.

@lindseymoore lindseymoore requested a review from mcmorisi May 30, 2025 23:49
Copy link
Collaborator

@mcmorisi mcmorisi 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 a few more changes!

Comment on lines +172 to +190
~~~~~~~~~~~~~

A tie occurs when two or more documents have identical values in the
field you are using to sort your results. MongoDB does not guarantee
order if ties occur.

For example, in the sample data, there is a tie for ``enrollment`` in
the following documents:

.. code-block:: none
:copyable: false

{"title":"World Fiction","enrollment":35}
{"title":"Plate Tectonics","enrollment":35}

You can sort on additional fields to resolve ties in the original sort.
If you want to guarantee a specific order for documents, select sort fields
that do not result in ties.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that this section doesn't discuss Go-specific implementation details, I think instead of this section we can have something like "To learn more about sort behavior, see the cursor.sort() documentation in the {+mdb-server+} manual"

Copy link
Collaborator Author

@lindseymoore lindseymoore Jun 2, 2025

Choose a reason for hiding this comment

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

I looked at the linked page, and the page does not explicitly mention handling ties. I also don't want to confuse the reader since the cursor.sort() syntax is slightly different. For these reasons, I'll leave as is!

@lindseymoore lindseymoore merged commit ef26268 into mongodb:comp-cov Jun 2, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants