[CDRIVER-1522] Add mongoc_collection_find_with_opts Created: 06/Sep/16  Updated: 13/Jan/17  Resolved: 21/Sep/16

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: 1.4.0
Fix Version/s: 1.5.0

Type: Bug Priority: Major - P3
Reporter: A. Jesse Jiryu Davis Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by DRIVERS-347 Support deprecated "modifiers" FindOp... Closed
is depended on by PHPC-783 Use mongoc_collection_find_with_opts(... Closed
is depended on by CDRIVER-1528 Deprecate mongoc_collection_find Closed
is depended on by CDRIVER-1539 Consider additional "opts" validation Closed
Related
related to CDRIVER-1554 Redundant handling of $query in _tran... Closed
is related to CDRIVER-1751 mongoc_collection_find_with_opts() sh... Closed
Epic Link: Flexible opts
Backwards Compatibility: Fully Compatible

 Description   

New function takes options expressed as a free-form BSON document. Paves the way to a more flexible future API in which we can add support for arbitrary options without API expansions.

mongoc_cursor_t *
mongoc_collection_find_with_opts (mongoc_collection_t       *collection,
                                  const bson_t              *filter,
                                  const bson_t              *opts,
                                  const mongoc_read_prefs_t *read_prefs);

The plan:

  1. mongoc_collection_find_with_opts translates to old-style _mongoc_cursor_new call with all the old parameters - done
  2. readConcern is prohibited - specs say drivers MAY allow it per-operation, we only allow it to be set on client, db, and collection, so let's not support it per-operation right now, and enforce that choice - done
  3. Test mongoc_collection_find_with_opts for the same scenarios as mongoc_collection_find - ensure it handles all the same server-specific cases (wire version, OP_QUERY vs "find" command, mongos vs mongod) - done
  4. Add "filter" and "opts" to mongoc_cursor_t struct. mongoc_collection_find_with_opts copies its "filter" and "opts" parameters to these fields of the mongoc_cursor_t - done
  5. When "filter" and "opts" are set, use them in the "find" command path instead of constructing from old fields in the mongoc_cursor_t - done
  6. When "filter" and "opts" are set, rewrite the OP_QUERY path to use them instead of the old fields (the work done for Step 1 is reusable here) - done
  7. Deal with maxAwaitTimeMS and test it with mongoc_collection_find_with_opts - done
  8. _mongoc_cursor_new drops its qflags, skip, limit, batch_size, fields, and read_concern parameters, gains an "opts" parameter. mongoc_collection_find_with_opts now calls it directly. mongoc_collection_find translates from its parameters to an "opts" document; e.g., batch_size=1 becomes {batchSize: 1}. Other _mongoc_cursor_new callers are also updated to create "opts" documents. - done
  9. Test that unrecognized opts fields are sent to server unmodified - done
  10. Delete old fields from mongoc_cursor_t struct: fields, write_concern, flags, skip, limit, batch_size, max_await_time_ms - done
  11. Update docs and NEWS - done
  12. Update signature for consistency with mongoc_collection_count_with_opts: (collection, filter, opts, read_prefs) - done
  13. Make the common strings like "awaitData" into macros - done
  14. Call _mongoc_cursor_new_with opts internally from mongoc_client_command, mongoc_client_find_databases, mongoc_collection_find, mongoc_cursor_new_from_command_reply, mongoc_database_find_collections - done
  15. Test _mongoc_n_return some more, it's been a bug factory in the past - done


 Comments   
Comment by Githook User [ 05/Jan/17 ]

Author:

{u'username': u'ooglek', u'name': u'Peter Beckman', u'email': u'beckman@angryox.com'}

Message: CDRIVER-1522 CDRIVER-1970 update define constants for "find" opts to be unique
Branch: r1.5
https://github.com/mongodb/mongo-c-driver/commit/0b872895b58a249773aec5b4ace902db7d75d675

Comment by Githook User [ 05/Jan/17 ]

Author:

{u'username': u'ooglek', u'name': u'Peter Beckman', u'email': u'beckman@angryox.com'}

Message: CDRIVER-1522 CDRIVER-1970 update define constants for "find" opts to be unique
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/6dd99265127d45104bb722062e3f4058b2cf1f9a

Comment by Githook User [ 27/Nov/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 use llabs for int64 in mongoc-cursor.c
Branch: r1.5
https://github.com/mongodb/mongo-c-driver/commit/a13dc52c901cd34c999ac32b1a6379f7d11c02de

Comment by Githook User [ 17/Nov/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 use llabs for int64 in mongoc-cursor.c
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/e49f133ae52c19c781f7fc037894c53d12e5217c

Comment by Hannes Magnusson [ 17/Nov/16 ]

This commit introduced new warning:

[2016/11/16 15:49:44.930] src/mongoc/mongoc-cursor.c:1784:34: error: absolute value function 'labs' given an argument of type 'int64_t' (aka 'long long') but has parameter of type 'long' which may cause truncation of value [-Werror,-Wabsolute-value]
[2016/11/16 15:49:44.930]    if (limit && cursor->count >= labs (limit)) {
[2016/11/16 15:49:44.930]                                  ^
[2016/11/16 15:49:44.930] src/mongoc/mongoc-cursor.c:1784:34: note: use function 'llabs' instead
[2016/11/16 15:49:44.930]    if (limit && cursor->count >= labs (limit)) {
[2016/11/16 15:49:44.930]                                  ^~~~
[2016/11/16 15:49:44.930]                                  llabs
[2016/11/16 15:49:44.935] 1 error generated.

See https://evergreen.mongodb.com/task/mongo_c_driver_clang38_i386_debug_compile_af74cb12269fc0e13b41f6b957c24f5a6d872a27_16_11_16_23_11_27

Comment by Githook User [ 21/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 thoroughly test cursor n_return
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/9d16fad50f941d1480d530f56c3c54109242db19

Comment by Githook User [ 21/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 use cursor_new_with_opts internally
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/70337e2c92d0da276b3728fec96521793c0bd8ef

Comment by Githook User [ 14/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 don't send maxAwaitTimeMS to server
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/a8357475f52eb8983a762f2e7ba180868830f4f3

Comment by Githook User [ 14/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 define constants for "find" opts
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/b14c511dbb756ecb19f2decb59f59fbe7541d863

Comment by Githook User [ 14/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 reorder cursor_new_with_opts signature

Update signature for consistency with mongoc_collection_count_with_opts:
(collection, filter, opts, read_prefs)
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/c7153978ba52e026fe777355cae139c01ba61761

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 reorder find_with_opts signature

Update signature for consistency with mongoc_collection_count_with_opts:
(collection, filter, opts, read_prefs)
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/bc1163ad6b01bfb6a2f332d17acd96c823818622

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 update find docs

Two changes in particular. First, $-modifiers once again have the "$"
stripped for "find" commands, and added for OP_QUERY messages. I'd
removed this behavior a few commits ago and restored it in 63a9d42e.

Second, "filter" is no longer treated specially by the old "find()", so
you don't have to do special things to query on a "filter" field.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/27cad5c3cf6b3df60e579e20939d76242a780e23

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 test & fix query and flags edge cases
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/0c12cc6e3f9e109ec118e143dbde688f58585dca

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 fix slave_ok flag logic
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f42cb2694863b041cdc033101161b192493bd34c

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 remove mongoc_cursor_t's old fields
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f066f14e7c9862a843f54700c3f3f36fe11fba80

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 always init cursor with "opts"
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f6c6085076415150ad97fa4d0950ea991d420971

Comment by Githook User [ 13/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 add topology-type getter
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f4cfb6eb708913a1795090faecbe3196fb7b5c58

Comment by Githook User [ 12/Sep/16 ]

Author:

{u'username': u'bjori', u'name': u'Hannes Magnusson', u'email': u'bjori@php.net'}

Message: Revert "CDRIVER-1522 del example code that won't build yet"

This reverts commit a497587867fe28dac954d0244ca6fb4bfc5e4a38.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/2115e6dc3ef927a2cbe8c598471210100841335a

Comment by Githook User [ 10/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 support maxAwaitTimeMS in "opts"
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/5358a712c95368eb86871a34567a9e8b8ae19694

Comment by Githook User [ 10/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 use filter and opts for OP_QUERY
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/b81f1b196fb822d5c104ecc42b01ab6e2ceec56f

Comment by Githook User [ 10/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 update find_with_opts tests
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/1d7fd846c8963ed395da187c656357c3c451b487

Comment by Githook User [ 09/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 update mongoc_collection_find_with_opts doc
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f1bd2107dd76022e06d97372718466aeb4fa2e62

Comment by Githook User [ 09/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 make cursor getters/setters use opts
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/fe43fddd14ef87c289cc49ba513a3445ef8f3920

Comment by Githook User [ 09/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 use filter and opts for "find" cmd
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/867fe6c8d52060e5e32bffefacccdd0efe2b67fa

Comment by Githook User [ 08/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 test unknown $-modifier with OP_QUERY
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/18cb2ba7a456b3320758c4a6b53a924b510ec691

Comment by Githook User [ 08/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 add mongoc_collection_find_with_opts
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/69ab5ff18a770568e15af0f6bbacbcd58f8891ac

Comment by Githook User [ 08/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 check read prefs in _mongoc_cursor_new
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/bb9d1ccb2f42cafdd552ecac49257f51be7a8f3e

Comment by Githook User [ 07/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: support negative nToReturn in mock server

This will help test mongoc_collection_find_with_opts, CDRIVER-1522.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/e15ff9fa69fb3c430c15e8b674a10f8af7aa7990

Comment by Githook User [ 07/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 del example code that won't build yet
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/a497587867fe28dac954d0244ca6fb4bfc5e4a38

Comment by Githook User [ 07/Sep/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1522 declare fns taking flexible BSON opts
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/37719eef990b0a191cdeff8cc161e585e6e52133

Comment by Bernie Hackett [ 06/Sep/16 ]

It's not actually a whitelist. Keys not in _MODIFIERS pass through unchanged:

Traceback (most recent call last):
  File "testcmd.py", line 8, in <module>
    list(c.foo.collection.find(modifiers={'foo': True}))
  File "/home/behackett/work/mongo-python-driver/pymongo/cursor.py", line 1090, in next
    if len(self.__data) or self._refresh():
  File "/home/behackett/work/mongo-python-driver/pymongo/cursor.py", line 1012, in _refresh
    self.__read_concern))
  File "/home/behackett/work/mongo-python-driver/pymongo/cursor.py", line 905, in __send_message
    helpers._check_command_response(doc['data'][0])
  File "/home/behackett/work/mongo-python-driver/pymongo/helpers.py", line 210, in _check_command_response
    raise OperationFailure(msg % errmsg, code, response)
pymongo.errors.OperationFailure: Failed to parse: { find: "collection", filter: {}, foo: true }. Unrecognized field 'foo'.

Comment by Jeremy Mikola [ 06/Sep/16 ]

FWIW, PHPC's Query constructor takes a modifiers option for this purpose, and also because we didn't want to support some options like hint in advance of them making it into the CRUD spec.

If I'm reading the PyMongo code correctly, the translation is only done for known modifiers. To be clear, is that (whitelist) what you're suggesting for libmongoc as well? Just want to be sure that "foo" above is a placeholder for a known modifier and not just anything

Comment by Bernie Hackett [ 06/Sep/16 ]

That looks like what we do in PyMongo.

https://github.com/mongodb/mongo-python-driver/blob/3.3.0/pymongo/message.py#L143-L181

Comment by A. Jesse Jiryu Davis [ 06/Sep/16 ]

behackett what do you think about Jeremy's question above? If someone calls mongoc_collection_find_with_opts with opts {foo: true} and we're talking to MongoDB 3.2+, do we send this command?:

{find: "collection", filter: {}, foo: true}

And for older servers, an OP_QUERY with "$" added to "foo" ?:

{$query: {}, $foo: true}

I think we should do this, to ensure support for future "find" command features and to ensure we didn't miss any obscure OP_QUERY features.

Comment by Jeremy Mikola [ 06/Sep/16 ]

$-prefixed options are prohibited, to avoid confusion about e.g. $orderby and "sort" - all options should use modern names

Test that unrecognized opts fields are sent to server unmodified

Do these two statements conflict? For legacy OP_QUERY usage, would we simply stuff $-prefixed modifiers into the first argument (i.e. filter document) and wrap the filter with $query as we presently do with mongoc_collection_find()?

Generated at Wed Feb 07 21:12:47 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.