feat: add explain support #2626
Conversation
HanaPearlman
force-pushed
the
NODE-2853/3.6/explain
branch
from
c45ef93
to
3abf006
Compare
| @@ -310,6 +309,7 @@ const DEPRECATED_FIND_OPTIONS = ['maxScan', 'fields', 'snapshot', 'oplogReplay'] | |||
| * @param {boolean} [options.noCursorTimeout] The server normally times out idle cursors after an inactivity period (10 minutes) to prevent excess memory use. Set this option to prevent that. | |||
| * @param {object} [options.collation] Specify collation (MongoDB 3.4 or higher) settings for update operation (see 3.4 documentation for available fields). | |||
| * @param {boolean} [options.allowDiskUse] Enables writing to temporary files on the server. | |||
| * @param {string|boolean} [options.explain] The verbosity mode for the explain output. | |||
| * @param {ClientSession} [options.session] optional session to use for this operation | |||
There was a problem hiding this comment.
I was not sure the best way to communicate what the valid values of options.explain are. We could list the valid strings and how the boolean values will be interpreted, which is what we do for readPreference, or we could link to some server documentation, which is what we do for hint.
There was a problem hiding this comment.
I did suggest lower to put the strings in, if we put the strings in VSCode can suggest the strings to devs 🤷 but then it might error if there's a new string later.. Maybe the link is best
There was a problem hiding this comment.
If we go ahead with using the enum from 4.0 to check that the string passed in is a valid verbosity, then it will error if there's a new verbosity string added later anyway.
Since we went ahead with doing the valid verbosity check in 4.0, my thought is to go ahead and do that here as well, and list the strings within the type. More work for us later if a new string is added, though.
|
|
||
| // If an explain operation was executed, don't process the server results | ||
| if (this.explain) return callback(undefined, r.result); | ||
|
|
There was a problem hiding this comment.
Got rid of deleteCallback and updateCallback in common_functions and moved the body of the functions into the respective operation classes; this way we can reference this.explain (and it is consistent with master).
HanaPearlman
force-pushed
the
NODE-2853/3.6/explain
branch
2 times, most recently
from
b395273
to
3233d5e
Compare
| @@ -310,6 +309,7 @@ const DEPRECATED_FIND_OPTIONS = ['maxScan', 'fields', 'snapshot', 'oplogReplay'] | |||
| * @param {boolean} [options.noCursorTimeout] The server normally times out idle cursors after an inactivity period (10 minutes) to prevent excess memory use. Set this option to prevent that. | |||
| * @param {object} [options.collation] Specify collation (MongoDB 3.4 or higher) settings for update operation (see 3.4 documentation for available fields). | |||
| * @param {boolean} [options.allowDiskUse] Enables writing to temporary files on the server. | |||
| * @param {string|boolean} [options.explain] The verbosity mode for the explain output. | |||
| * @param {ClientSession} [options.session] optional session to use for this operation | |||
There was a problem hiding this comment.
I did suggest lower to put the strings in, if we put the strings in VSCode can suggest the strings to devs 🤷 but then it might error if there's a new string later.. Maybe the link is best
HanaPearlman
force-pushed
the
NODE-2853/3.6/explain
branch
2 times, most recently
from
b0ed9e9
to
8d424e5
Compare
HanaPearlman
force-pushed
the
NODE-2853/3.6/explain
branch
from
8d424e5
to
86341f9
Compare
| r.modifiedCount = r.result.nModified != null ? r.result.nModified : r.result.n; | ||
| r.upsertedId = | ||
| Array.isArray(r.result.upserted) && r.result.upserted.length > 0 | ||
| ? r.result.upserted[0] // FIXME(major): should be `r.result.upserted[0]._id` |
There was a problem hiding this comment.
I wonder if this fixme is still relevant..
HanaPearlman
force-pushed
the
NODE-2853/3.6/explain
branch
from
86341f9
to
3a2df97
Compare
Port explain support to 3.6.
The logical changes are the same, but some things had to occur in different places: Since
OperationBaseis the only shared class between all of the explainable ops, theexplainproperty is onOperationBase, anddecorateWithExplainis used in a few more places.