[SERVER-12263] Support $elemMatch with DBRefs that may contain additional properties Created: 06/Jan/14  Updated: 26/Jan/17  Resolved: 07/Feb/14

Status: Closed
Project: Core Server
Component/s: Querying
Affects Version/s: 2.5.4
Fix Version/s: 2.6.0-rc0

Type: Bug Priority: Major - P3
Reporter: Jeremy Mikola Assignee: Benety Goh
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File server12263.js    
Issue Links:
Related
related to SERVER-23786 Server allows insertion and update of... Backlog
related to SERVER-10777 Allow JSON Parser to support addition... Closed
related to DOCS-5135 Document that DBRef objects allow ext... Closed
is related to SERVER-12191 CanonicalQuery::canonicalize() reject... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Participants:

 Description   

tl;dr:

  • DBRefs are objects whose first two fields must be $ref and $id (in that order). An optional $db field, if present, must appear third. Other fields may follow (they can't have a $ prefix, of course).
  • $elemMatch should have consistent behavior, regardless of whether its argument or the value it is matching against look like DBRef objects.

The current implementation simply treats an $elemMatch on a DBRef as an equality match, which doesn't seem correct. Consider the following, which uses the latest nightly:

> db.runCommand('buildInfo').gitVersion
8bad5a8633eeca3bcbb975aabf268db9921f2c57
 
> db.people.insert({_id: 1})
Insert WriteResult({ "ok" : 1, "n" : 1 })
 
> db.people.insert({_id: 2, friends: [ {$ref: "people", $id: 1, $db: "test", _doctrine_class_name: "Documents\People" } ]})
Insert WriteResult({ "ok" : 1, "n" : 1 })
 
> db.people.insert({_id: 3, friends: [ {$ref: "people", $id: 2, $db: "test" } ]})
Insert WriteResult({ "ok" : 1, "n" : 1 })

First, I create three documents in the test.people collection. The middle document has an extra field in its DBRef object, which is something Doctrine ODM has done for years to discriminate references to super-classes. Based on the DBRef docs, they're only objects that required $ref and $id as the first two fields with an optional $db to follow. There was never a restriction on additional fields, even though the shell likes to hide them (as it does $db).

> db.people.find()
{ "_id" : 1 }
{ "_id" : 2, "friends" : [ DBRef("people", 1) ] }
{ "_id" : 3, "friends" : [ DBRef("people", 2) ] }

The example below demonstrates the shortcoming of treating $elemMatch as an equality comparison, and relying on _isDBRefDocument() for the detection:

> db.people.findOne({friends: { $elemMatch: { $ref: "people", $db: "test" }}})
2014-01-06T11:45:33.768-0500 error: {
	"$err" : "Can't canonicalize query: BadValue unknown operator: $ref",
	"code" : 17287
} at src/mongo/shell/query.js:131
> db.people.findOne({friends: { $elemMatch: { $ref: "people", $id: 1 }}})
null
> db.people.findOne({friends: { $elemMatch: { $ref: "people", $id: 1, $db: "test" }}})
null
> db.people.findOne({friends: { $elemMatch: { $ref: "people", $id: 1, $db: "test", _doctrine_class_name: "Documents\People" }}})
2014-01-06T11:46:09.520-0500 error: {
	"$err" : "Can't canonicalize query: BadValue unknown operator: $ref",
	"code" : 17287
} at src/mongo/shell/query.js:131

In the first and last example, we get the original error because $elemMatch doesn't recognize its argument as a DBRef and apply the patch logic. But even with the patch, the equality comparison fails to match anything in the middle two examples, since the DBRef in the database does have an extra _doctrine_class_name field within.

Repeating the test with the third example document, which doesn't include the extra Doctrine field:

> db.people.findOne({friends: { $elemMatch: { $ref: "people", $db: "test" }}})
2014-01-06T11:46:37.305-0500 error: {
	"$err" : "Can't canonicalize query: BadValue unknown operator: $ref",
	"code" : 17287
} at src/mongo/shell/query.js:131
> db.people.findOne({friends: { $elemMatch: { $ref: "people", $id: 2 }}})
null
> db.people.findOne({friends: { $elemMatch: { $ref: "people", $id: 2, $db: "test" }}})
{ "_id" : 3, "friends" : [ DBRef("people", 2) ] }

We still have the same shortcomings, although the equality match in the last example does happen to work. I'm still not keen on that behavior because this is nothing like a real $elemMatch evaluation.



 Comments   
Comment by Jeremy Mikola [ 12/Feb/14 ]

benety.goh: Just tested master and the above cases all work. Thanks for the fix.

Comment by Githook User [ 07/Feb/14 ]

Author:

{u'username': u'benety', u'name': u'Benety Goh', u'email': u'benety@mongodb.com'}

Message: SERVER-12263 support incomplete DBRef under elemMatch
Branch: master
https://github.com/mongodb/mongo/commit/4ad2b23b83c96a272cb1a5c4f33ff15971ff69b9

Comment by Benety Goh [ 07/Feb/14 ]

hope this last patch submitted to master works for the doctrine test cases.

Comment by Jeremy Mikola [ 06/Feb/14 ]

How so? The original description always included the following case:

db.people.findOne({friends: { $elemMatch: { $ref: "people", $db: "test" }}})

Likewise, scotthernandez's test cases pre-date any work on the ticket.

Comment by Benety Goh [ 06/Feb/14 ]

Can we make support for incomplete DBRefs in $elemMatch another ticket? That wasn't in the scope of the original description that you wrote.

Comment by Jeremy Mikola [ 05/Feb/14 ]

I noticed that this commit included a test for $in where the DBRef was missing fields, which seems well and good ($in should have an array of valid DBRefs in that case), but there is no test for $elemMatch with an incomplete DBRef, which is what should be supported per Scott's earlier comment.

Comment by Jeremy Mikola [ 05/Feb/14 ]

benety.goh: I compiled https://github.com/mongodb/mongo/commit/bd801007fa10cadfb25c1b211249a17fb0b97ac5, which includes your commits above, and some broken behavior remains:

db.people.drop();
db.people.insert({_id: 1});
db.people.insert({_id: 2, friends: [ {$ref: "people", $id: 1, $db: "test", _doctrine_class_name: "Documents\People" } ]});
db.people.insert({_id: 3, friends: [ {$ref: "people", $id: 2, $db: "test" } ]});
db.people.findOne({friends: { $elemMatch: { $ref: "people", $id: 1, $db: "test", _doctrine_class_name: "Documents\People" }}});
db.people.findOne({friends: { $elemMatch: { $ref: "people", $db: "test" }}});
db.people.findOne({friends: { $elemMatch: { $id: 2 }}});

The first query succeeds, so that bit was fixed, but the second and third queries still fail.

Comment by Githook User [ 05/Feb/14 ]

Author:

{u'username': u'benety', u'name': u'Benety Goh', u'email': u'benety@mongodb.com'}

Message: SERVER-12263 relax ordering requirement of DBRef fields in query
Branch: master
https://github.com/mongodb/mongo/commit/98e57eca44dd47e0d3f20df2da1d534086a30b23

Comment by Githook User [ 05/Feb/14 ]

Author:

{u'username': u'benety', u'name': u'Benety Goh', u'email': u'benety@mongodb.com'}

Message: SERVER-12263 relax DBRef parsing requirements to support additional fields after ref and id
Branch: master
https://github.com/mongodb/mongo/commit/b8fc587a77801725ca001b14d5928e0d5868be00

Comment by Scott Hernandez (Inactive) [ 22/Jan/14 ]

This needs to work.

// not using extra fields, but just part of the DBRef
db.people.findOne({friends: { $elemMatch: { $id: 2 }}})
 
// extra fields
db.people.findOne({friends: { $elemMatch: { $id: 2, foo:1 }}})

Generated at Thu Feb 08 03:28:03 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.