[SERVER-58670] Modernize DBClientBase query interface to avoid OP_QUERY-derived characteristics Created: 19/Jul/21  Updated: 29/Oct/23  Resolved: 23/Aug/21

Status: Closed
Project: Core Server
Component/s: Internal Client
Affects Version/s: None
Fix Version/s: 5.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: David Storch Assignee: Irina Yatsenko (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-59512 Introduce new find API to DBClientBas... Closed
Backwards Compatibility: Fully Compatible
Sprint: QE 2021-08-09, QE 2021-08-23, QE 2021-09-06
Participants:

 Description   

The Ultimate Goal: migrate DBClientBase towards the interface similar to https://github.com/mongodb/specifications/blob/master/source/crud/crud.rst#read. For example, a query() API will take namespace, BSON filter and a special type with settings, rather than the current API that takes a Query with the filter and some settings embedded into it and other settings being passes in as additional parameters. As we do that, the Query type should be replaced by QueryOptions with statically typed fields (some of which might be BSON as required by the particular settings).

The Challenge: the Query type wraps around a BSON object that might contain both query filter and some query settings. For a few common settings, such as sort and hint, there are setters on Query type that integrate the settings into the internal BSON object in a well-defined way, but Query also provides implicit constructor from BSON so there is no compiler check for what it might be get wrapped around. As a result, when it comes to processing a query, the code has to inspect Query's public 'obj' member and be prepared to deal with complicated BSON. We also suspect that some of the settings passed around have become mute after OP_QUERY deprecation and could be removed but it's hard to verify when opaque BSON is used to pass them around.

Due to wide usage of DBClientBase and Query types, a single change to reach the goal stated above isn't feasible and the proposed progression is as follows:

https://github.com/10gen/mongo/pull/215 did some preliminary cleanups.

1. Split the query argument in query() APIS into separate 'filter' (as BSONObj) and 'querySettings' (as Query) arguments.
2. Replace Query constructor from BSON with an explicit factory so that all callsites that use the constructor become easily searchable (fromBSONDeprecated())
3. Replace as many callsites that create Query from BSON with the default query construction, followed by calling individual setters for each option (NB: this means we might copy the internal BSON object a few times rather than create it once but, hopefully, this won't cause a noticeable perf impact). Add new setters as necessary (some might be removed later, when we confirm they've been fully deprecated).
4. Remove QUERY macro.

The steps 1-4 have been implemented in https://github.com/10gen/mongo/pull/292.

5. Investigate and remove/replace the remaining four callers of fromBSONDeprecated():

  • (src/mongo/client/dbclient_rs.cpp) in DBClientReplicaSet::say under if (lastOp == dbQuery) check – is this code still reachable?
  • (src/mongo/client/dbclient_rs.cpp) in DBClientReplicaSet::call under if (toSend.operation() == dbQuery) check – is this code still reachable?
  • (src/mongo/db/repl/apply_ops.cpp) in _checkPrecondition – should/could preconditions use different format?
  • (src/mongo/scripting/mozjs/mongo.cpp) in MongoBase::Functions::find::call – should we still be supporting 'query/$query' here?

6. When all Query construction from raw BSON is gone, the type will only handle a known fixed set of query properties, so it will become possible to replace the internal BSON field 'obj' with statically typed fields, and callers that inspect 'obj' will be able to switch to using specific statically typed accessors instead. There are currently 7 callers like this (search for getFullSettingsDeprecated()).
7. Rename the Query type into QueryOptions (or FindOptions) and incorporate into it the other options that are currently passed to the query() APIs as separate arguments.
8. Review and cleanup src/mongo/db/query/query_request_helper.cpp. There should be no vestiges of OP_QUERY left at this point.



 Comments   
Comment by Vivian Ge (Inactive) [ 06/Oct/21 ]

Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you!

Comment by Irina Yatsenko (Inactive) [ 23/Aug/21 ]

Filed https://jira.mongodb.org/browse/SERVER-59512 to track the remaining work.

Comment by Githook User [ 23/Aug/21 ]

Author:

{'name': 'Irina Yatsenko', 'email': 'irina.yatsenko@mongodb.com', 'username': 'IrinaYatsenko'}

Message: SERVER-58670 Tighten up what kind of BSON the 'Query' type can be wrapped around

This refactor includes:
Remove dead code from 'Query' type and reduce it public interface.
Split query argument in query/update/removed methods into filter BSON and settings (which are still passed around as 'Query' type).
Remove Query(string) constructors.
Remove most callers of 'Query(const BSONObj&)'.
Replace public 'Query(const BSON&)' and 'Query.obj' with an explicit factory method and a getter.
Branch: master
https://github.com/mongodb/mongo/commit/df329d8f46e1485dd5d70379f9c48bf4175f0d5a

Comment by Githook User [ 23/Aug/21 ]

Author:

{'name': 'Irina Yatsenko', 'email': 'irina.yatsenko@mongodb.com', 'username': 'IrinaYatsenko'}

Message: SERVER-58670 Tighten up what kind of BSON the 'Query' type can be wrapped around
Branch: master
https://github.com/10gen/mongo-enterprise-modules/commit/f3b098266be508b049fa36dcb6f3671830374e20

Comment by Githook User [ 14/Aug/21 ]

Author:

{'name': 'Irina Yatsenko', 'email': 'irina.yatsenko@mongodb.com', 'username': 'IrinaYatsenko'}

Message: SERVER-58670 Modernize DBClientBase query interface to avoid OP_QUERY-derived characteristics
Branch: master
https://github.com/mongodb/mongo/commit/1f64d42977db0572b08d7ab19133bc3f21323ce0

Comment by David Storch [ 29/Jul/21 ]

We should also look into deleting DBClientBase::findN() as part of this ticket. We noticed that this method has very few callers, and those callers could easily be migrated onto the regular DBClientBase::query() API.

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