Skip to content

DRIVERS-3064 Add support for the rawData command option #1814

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 10 commits into from
Jul 17, 2025

Conversation

qingyang-hu
Copy link
Contributor

Add support for the new rawData command option for the following CRUD operations:

  • aggregate
  • bulkWrite (collection and client)
  • count
  • countDocuments
  • deleteMany
  • deleteOne
  • distinct
  • estimatedDocumentCount
  • find
  • findOne
  • findAndModify
  • insertMany
  • insertOne
  • replaceOne
  • updateMany
  • updateOne
  • listIndexes
  • dropIndexes
  • createIndexes
  • listCollections

Please complete the following before merging:

  • Update changelog.
  • Test changes in at least one language driver.
  • Test these changes against all server versions and topologies (including standalone, replica set, and sharded
    clusters).

@qingyang-hu qingyang-hu requested a review from alcaeus July 3, 2025 00:18
@qingyang-hu qingyang-hu marked this pull request as ready for review July 3, 2025 00:21
@qingyang-hu qingyang-hu requested review from a team as code owners July 3, 2025 00:21
@qingyang-hu qingyang-hu requested review from nbbeeken and removed request for a team July 3, 2025 00:21
@qingyang-hu qingyang-hu marked this pull request as draft July 3, 2025 15:51
@qingyang-hu qingyang-hu force-pushed the drivers3064 branch 5 times, most recently from fa93a58 to 0f8c6bd Compare July 3, 2025 23:55
@qingyang-hu qingyang-hu marked this pull request as ready for review July 3, 2025 23:56
- description: "listCollections with rawData option"
runOnRequirements:
- minServerVersion: "8.2.0"
auth: false
Copy link
Member

Choose a reason for hiding this comment

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

Why do these tests require authentication to be disabled? My impression is that they should work just fine with or without auth. If so, let's remove the requirement, or alternatively document why this is necessary in a comment for future reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

8.2 does not support the rawDate with listCollections:

(Unauthorized) not authorized on database0 to execute command

@alcaeus alcaeus removed the request for review from nbbeeken July 8, 2025 11:43
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

@durran
Copy link
Member

durran commented Jul 10, 2025

Node impl mongodb/node-mongodb-native#4581. LGTM.

- commandStartedEvent:
command:
listCollections: 1
filter: {}
Copy link
Member

Choose a reason for hiding this comment

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

While testing for a potential auth failure with PHP, I noticed that this test initially failed because PHP does not send an empty filter field in the command. Since we're not testing for any filters, consider removing this line here and in the test below. Alternatively, use $unsetOrMatches as both are equivalent.

Copy link
Member

Choose a reason for hiding this comment

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

I'm in support of the above change. Although our CRUD spec denotes that find() takes a required filter parameter, it says nothing about the outgoing command. And if we consult the find command docs we'll see:

Optional. The query predicate. If unspecified, then all documents in the collection will match the predicate.

I think $$unsetOrMatches: {} makes sense in this case.

Copy link
Member

@alcaeus alcaeus 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 the filter changes

@qingyang-hu qingyang-hu merged commit 7bff2f5 into mongodb:master Jul 17, 2025
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.

4 participants