-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PHPLIB-1192: Remove useCursor option in aggregate #1130
Conversation
} | ||
$cmd['cursor'] = isset($this->options["batchSize"]) | ||
? ['batchSize' => $this->options["batchSize"]] | ||
: new stdClass(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume always supplying a cursor
option for the explain
option is kosher?
Quoting Aggregate: Example from the server docs:
MongoDB 3.6 removes the use of aggregate command without the cursor option unless the command includes the explain option. Unless you include the explain option, you must specify the cursor option.
It doesn't say anything about explain
prohibiting a cursor
option, but cursor
does seem to be ignored. Do you think we should gate this with a ! $this->isExplain
condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave the option in unless explain
specifically prohibits it - this is consistent with other commands where we also send batchSize
as part of an explain
.
``result`` array from the command response document. In both cases, the return | ||
value will be :php:`Traversable <traversable>`. | ||
:phpmethod:`MongoDB\\Collection::aggregate()`'s returns a | ||
:php:`MongoDB\\Driver\\Cursor <class.mongodb-driver-cursor>` object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this applies regardless of whether explain: true
is used. SGTM.
9ba7b15
to
f56ae4e
Compare
f56ae4e
to
5631bd8
Compare
PHPLIB-1192
I came across this while working on codecs, and removing this legacy logic makes the logic significantly simpler.