[SERVER-40270] Distinct command no longer supports $comment Created: 22/Mar/19  Updated: 27/Oct/23  Resolved: 26/Mar/19

Status: Closed
Project: Core Server
Component/s: Querying
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Shane Harvey Assignee: Backlog - Query Team (Inactive)
Resolution: Works as Designed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by PYTHON-1786 PyMongo sends "$comment" with distinc... Closed
Problem/Incident
is caused by SERVER-39256 Implement translation for distinct co... Closed
Assigned Teams:
Query
Operating System: ALL
Participants:

 Description   

SERVER-39256 broke $comment support for the distinct command:

$ mongo
MongoDB shell version v4.0.1
connecting to: mongodb://127.0.0.1:27017
MongoDB server version: 4.1.9-87-g40f54965f9
WARNING: shell and server versions do not match
> db.runCommand({distinct:"test", key: "x", $comment: "dsf"})
{
	"ok" : 0,
	"errmsg" : "BSON field 'distinct.$comment' is an unknown field.",
	"code" : 40415,
	"codeName" : "Location40415"
}



 Comments   
Comment by Shane Harvey [ 26/Mar/19 ]

Thank you david.storch. I've changed PyMongo to send "comment" instead of "$comment" in PYTHON-1786. Older versions of PyMongo will error when attempting a Cursor.distinct() with a comment on MongoDB >= 4.1.10 like this:

>>> coll.find().distinct("x")
[1]
>>> coll.find(comment="my_comment").distinct("x")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pymongo/cursor.py", line 799, in distinct
    key, session=self.__session, **options)
  File "pymongo/collection.py", line 2666, in distinct
    collation=collation, session=session)["values"]
  File "pymongo/collection.py", line 244, in _command
    retryable_write=retryable_write)
  File "pymongo/pool.py", line 579, in command
    unacknowledged=unacknowledged)
  File "pymongo/network.py", line 150, in command
    parse_write_concern_error=parse_write_concern_error)
  File "pymongo/helpers.py", line 155, in _check_command_response
    raise OperationFailure(msg % errmsg, code, response)
pymongo.errors.OperationFailure: BSON field 'distinct.$comment' is an unknown field.

That's okay because users must upgrade their drivers before upgrading the server. Upgrading to PyMongo >=3.8 (to be released soon) will fix the issue:

>>> coll.find(comment="my_comment").distinct("x")
[1]
>>> pymongo.version
'3.8.0.dev0'

Comment by David Storch [ 26/Mar/19 ]

shane.harvey, the distinct command historically had a loose parser which accepted but ignored any unknown field names. This is why $comment used to work for distinct in addition to comment. Nick's work for SERVER-39256 tightened up the parsing (by creating an IDL parser for the distinct command internally). The logs and system.profile are built to report all command parameters, so for the most part all we have to do on the server is tolerate the presence of the comment option. Since we ended up tolerating both comment and $comment server-side for distinct, there really was no difference between them.

That said, our intention going forward is to make sure that all commands support comment, the non-$-prefixed variant, as a top-level command parameter. This work is tracked by SERVER-29794. In general, commands will return an error if given $comment as a top-level command parameter. Unfortunately, $comment does exist as a separate concept within match expressions: https://docs.mongodb.com/manual/reference/operator/query/comment/#op._S_comment. This means that, in addition to the top-level comment parameter, the server will support commands such as the following:

 > db.c.find({a: {$eq: 1}, $comment: "find a == 1"})
 > db.c.aggregate([{$match: {$comment: "project out everything except b"}}, {$project: {_id: 0, b: 1}}])

Hopefully that helps to clarify!

Comment by Shane Harvey [ 22/Mar/19 ]

I did a deep dive in the server history and found that:

  1. The find command "comment" option was added in SERVER-15230 MongoDB 2.7.7
  2. The aggregate "comment" option was added in SERVER-28128 MongoDB 3.5.5
  3. The distinct and count commands gained "comment" support in SERVER-28040 MongoDB 3.5.6 in this commit: https://github.com/mongodb/mongo/commit/c3341d179db1c36722284676d99cb1c664fb1821
  4. Finally, "support" for $comment on distinct was removed by the stricter parsing in SERVER-39256
    MongoDB 4.1.10

So PyMongo has been sending $comment since 2.6 and assumed it worked correctly because the field happens to also show up in the system.profile collection. I think we should fix PyMongo to send "comment" instead of "$comment".

I do have one question: since both "comment" and "$comment" (or any arbitrary field name) are present in the server logs and in system.profile, what is the difference between the two?

Comment by Shane Harvey [ 22/Mar/19 ]

Oh that's very interesting. Pymongo has been sending '$comment' with distinct since PyMongo 2.7 which added support for MongoDB 2.6. Do you know if $comment is also ignored on MongoDB 2.6?

Comment by David Storch [ 22/Mar/19 ]

Why do we believe that $comment is the right spelling? The name of the generic command parameter elsewhere for comment is just comment, with no dollar-prefixing. Are we sure this isn't a bug in the driver test?

Comment by Nicholas Zolnierz [ 22/Mar/19 ]

Thanks for catching this, Shane. I took a cursory look and it appears that the previous distinct parser handled 'comment' but not '$comment', and so I kept the same behavior as part of SERVER-39256. Since we now fail on unknown fields, it explains why your test triggered a failure. (Also side note - $comment with the previous distinct parsing has no effect)

Hopefully this is just a matter of changing the comment field name in the IDL.

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