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

DOCSP-41135: Specify Fields #8

Merged
merged 7 commits into from
Jul 18, 2024

Conversation

mcmorisi
Copy link
Collaborator

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41135
Staging - https://preview-mongodbmcmorisi.gatsbyjs.io/kotlin-sync/DOCSP-41135-specify-fields/read/project

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?

Copy link

netlify bot commented Jul 12, 2024

👷 Deploy request for docs-kotlin-sync pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 2a33eaa

Copy link

@shuangela shuangela left a comment

Choose a reason for hiding this comment

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

good work! a few small comments/questions

----------------

You can use a projection to specify which fields to include in a return
document, or to specify which fields to exclude. You cannot combine inclusion and

Choose a reason for hiding this comment

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

Q: I'm unclear on whether "You cannot combine inclusion and exclusion statements..." means that I can combine both only if I am excluding the _id field and nothing else (in my exclusion statement), or if I can combine both if I have an exclusion statement that excludes the _id field as well as excluding other fields.

Choose a reason for hiding this comment

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

C: Ok, reading further I now understand that the exclusion statement only applies to _id. You could potentially make that clearer earlier in the document.

Choose a reason for hiding this comment

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

C: This also confuses me because it seems like, based on my understanding, the only time you'd combine includes and excludes statements is to exclude the _id field, as all other fields are implicitly excluded when you use a includes statement? Could you clarify when someone would want to use both an includes and excludes that didn't just exclude _id?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is that a standard query will pull all the document's fields, and once you apply a projection to specify the inclusion of a single field, all other fields automatically get excluded (except "_id").

Someone shouldn't want to use both an includes and excludes. I can try addressing this confusion by pulling up the clarifying text from below the page into this mention up here.

Choose a reason for hiding this comment

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

yeah, I think moving the clarifying text up would be good.

.. code-block:: kotlin

val projection = Projection.fields(
Projections.include(<fieldName1>, <fieldName2>, ...),

Choose a reason for hiding this comment

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

Q: is there supposed to be a comma at the end of this line?

Suggested change
Projections.include(<fieldName1>, <fieldName2>, ...),
Projections.include(<fieldName1>, <fieldName2>, ...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, can remove

When specifying fields to include, you can also exclude the ``_id`` field from
the returned document.

The following example performs the same query as the preceding example, but

Choose a reason for hiding this comment

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

Suggested change
The following example performs the same query as the preceding example, but
The following example runs the same query as the preceding example, but

Exclude the ``_id`` Field
~~~~~~~~~~~~~~~~~~~~~~~~~

When specifying fields to include, you can also exclude the ``_id`` field from
Copy link

@shuangela shuangela Jul 12, 2024

Choose a reason for hiding this comment

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

Q: Can you only exclude the _id field when specifying fields to include, or can you exclude just the _id field without using any includes statements?

@mcmorisi mcmorisi requested a review from shuangela July 12, 2024 16:08
Copy link

@shuangela shuangela left a comment

Choose a reason for hiding this comment

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

lgtm

@mcmorisi mcmorisi requested review from a team and vbabanin and removed request for a team July 12, 2024 17:23
@tom-selander tom-selander requested review from rozza and removed request for vbabanin July 17, 2024 14:55
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM

@mcmorisi mcmorisi merged commit 53751bb into mongodb:master Jul 18, 2024
1 of 2 checks passed
mcmorisi added a commit that referenced this pull request Jul 18, 2024
(cherry picked from commit 53751bb)
mcmorisi added a commit that referenced this pull request Jul 18, 2024
(cherry picked from commit 53751bb)
@mcmorisi mcmorisi deleted the DOCSP-41135-specify-fields branch July 18, 2024 13:15
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.

3 participants