[SERVER-59302] Remove shell support for $query/query "wrapped" commands Created: 11/Aug/21  Updated: 29/Oct/23  Resolved: 17/Nov/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.3.0-rc0

Type: Improvement Priority: Minor - P4
Reporter: David Storch Assignee: David Storch
Resolution: Fixed Votes: 0
Labels: query-offsite, storch
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-29091 Stop putting $readPreference inside o... Open
Backwards Compatibility: Minor Change
Sprint: QE 2021-12-27, QE 2022-01-10, QE 2022-11-14, QE 2022-11-28
Participants:

 Description   

For reasons that I do not fully understand, but which presumably derive from the format once used for OP_QUERY commands, the shell supports the following:

MongoDB Enterprise > db.runCommand({$query: {ping: 1}})
{ "ok" : 1 }
MongoDB Enterprise > db.runCommand({query: {ping: 1}})
{ "ok" : 1 }

These are both identical to db.runCommand({ping: 1}). All three shell commands cause the shell to issue an OP_MSG command with the payload in the "unwrapped" form {ping: 1}. Neither the wrapped form nor OP_QUERY commands in general are supported by the 5.1 server, so the concept of a "wrapped" command exists exclusively in the legacy shell.

It appears that the shell has JS code in a number of places to handle commands that are wrapped inside query or $query fields. On the vanilla runCommand() path, the wrapped format is handled here:

https://github.com/mongodb/mongo/blob/7770140bab60c531ed4c20a40514a1a625b1cb5c/src/mongo/shell/session.js#L172-L178

There is really no reason for this behavior, as far as I'm aware. In order to simplify the code base, we should remove the shell's handling for the "wrapped" command format and migrate any tests using this format to the more typical "unwrapped" format. In other words, an instance of {$query: {ping: 1}} in a test should be simplified to {ping: 1}. I don't think we have to worry about the implications of this change for end users, as the mongo shell is deprecated for use by end users in favor of mongosh.



 Comments   
Comment by David Storch [ 17/Nov/22 ]

I've marked this as a minor backwards compatibility change because it could theoretically affect users of the legacy mongo shell. Any explicit uses of commands wrapped in query or $query and invoked with the shell's runCommand() helpers will no longer work, and will instead fail with a CommandNotFound error. Luckily there was never a good reason for external users to express the command with this wrapped format – this was an internal format related to read preference which would be generated internally when using helpers to configure the read preference.

Also note that the mongo shell is now intended only for internal testing. External users are expected to use mongosh instead. External users who migrated from mongo to mongosh when upgrading to recent versions of the server should experience no changes. Furthermore, mongosh never supported the wrapped "query"/"$query" format in the first place, so this change brings the behavior of the legacy shell into alignment with mongosh.

Comment by Githook User [ 17/Nov/22 ]

Author:

{'name': 'David Storch', 'email': 'david.storch@mongodb.com', 'username': 'dstorch'}

Message: SERVER-59302 Remove support in client code for commands wrapped with $query

The shell and internal client used to upconvert commands
wrapped in a "$query" or "query" to a well-formed OP_MSG.
This behavior is a holdover from when OP_QUERY was supported
and is no longer needed.
Branch: master
https://github.com/mongodb/mongo/commit/20d43f94ce5e943971904f65f8abff1e8b67521f

Comment by Ana Meza [ 04/Oct/22 ]

As per the Quick Wins Triage meeting, we are sending this back to the Triage queue for further discussions with the team since this ticket seems bigger than a quick win

Comment by David Storch [ 14/Jan/22 ]

I spent some time looking into this, and have a branch with some in-progress work that might be useful if we pick this up in the future. The work for this is fairly substantial – it's not just a matter of removing one or two checks for $query from shell code.

I believe I discovered the reason for why the shell still has plenty of code which allows commands to be wrapped in $query / query. It relates to the way that read preference was specified for the OP_QUERY RPC protocol. In particular, OP_QUERY commands had a "wrapped" form by which readPreference was passed:

{$query: <command body>, $readPreference: <read pref>}
 
OR
 
{query: <command body>, $readPreference: <read pref>}

The OP_MSG protocol, on the other hand, accepts $readPreference as a normal command parameter. A lot of this code is leftover from the days of OP_QUERY commands, even though the shell would generally upconvert this "wrapped" format to an OP_MSG with an "unwrapped" $readPreference argument.

I do think it's worth scheduling this task at some point order to get rid of the complexity tied to "wrapped" commands, as it is not necessary now that OP_QUERY has been removed from the code base.

Comment by David Storch [ 12/Aug/21 ]

I would vote for leaving this on the backlog. It could maybe be a quick win nomination, although it's hard to say how quick it will really be.

Generated at Thu Feb 08 05:46:54 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.