[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: |
|
||||||||
| 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:
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: 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: The shell and internal client used to upconvert commands | |||||
| 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:
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. |