Skip to content

PHPLIB-810: Always use count command for estimatedDocumentCount #926

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

Merged
merged 14 commits into from
May 19, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ source:
ref: bypassDocumentValidation
---
source:
file: apiargs-aggregate-option.yaml
file: apiargs-common-option.yaml
ref: comment
post: |
.. versionadded:: 1.3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ source:
file: apiargs-MongoDBCollection-common-option.yaml
ref: collation
---
source:
file: apiargs-common-option.yaml
ref: comment
post: |
.. versionadded:: 1.13
---
arg_name: option
name: hint
type: string|array|object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ source:
file: apiargs-MongoDBCollection-common-option.yaml
ref: collation
---
source:
file: apiargs-common-option.yaml
ref: comment
post: |
.. versionadded:: 1.13
---
arg_name: option
name: hint
type: string|array|object
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
source:
file: apiargs-common-option.yaml
ref: comment
post: |
.. versionadded:: 1.13
---
source:
file: apiargs-common-option.yaml
ref: maxTimeMS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ source:
ref: bypassDocumentValidation
---
source:
file: apiargs-aggregate-option.yaml
file: apiargs-common-option.yaml
ref: comment
---
source:
Expand Down
10 changes: 0 additions & 10 deletions docs/includes/apiargs-aggregate-option.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ post: |
:ref:`$out <agg-merge>` stages.
---
arg_name: option
name: comment
type: string
description: |
Users can specify an arbitrary string to help trace the operation through the
database profiler, currentOp, and logs.
interface: phpmethod
operation: ~
optional: true
---
arg_name: option
name: explain
type: boolean
description: |
Expand Down
16 changes: 16 additions & 0 deletions docs/includes/apiargs-common-option.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ operation: ~
optional: true
---
arg_name: option
name: comment
type: mixed
description: |
Enables users to specify an arbitrary comment to help trace the operation through
the database profiler, currentOp and logs. The default is to not send a value.

The comment can be any valid BSON type for server versions 4.4 and above.
Server versions between 3.6 and 4.2 only support string as comment,
and providing a non-string type will result in a server-side error.
Older server versions do not support comment for aggregate command at all,
and providing one will result in a server-side error.
interface: phpmethod
operation: ~
optional: true
---
arg_name: option
name: hint
type: string|array|object
description: |
Expand Down
15 changes: 9 additions & 6 deletions src/Operation/Aggregate.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,15 @@ class Aggregate implements Executable, Explainable
*
* * collation (document): Collation specification.
*
* * comment (string): An arbitrary string to help trace the operation
* through the database profiler, currentOp, and logs.
* * comment (mixed): Enables users to specify an arbitrary comment to help trace
* the operation through the database profiler, currentOp and logs. The
* default is to not send a value.
*
* The comment can be any valid BSON type for server versions 4.4 and above.
* Server versions between 3.6 and 4.2 only support string as comment,
* and providing a non-string type will result in a server-side error.
* Older server versions do not support comment for aggregate command at all,
* and providing one will result in a server-side error.
*
* * explain (boolean): Specifies whether or not to return the information
* on the processing of the pipeline.
Expand Down Expand Up @@ -170,10 +177,6 @@ public function __construct($databaseName, $collectionName, array $pipeline, arr
throw InvalidArgumentException::invalidType('"collation" option', $options['collation'], 'array or object');
}

if (isset($options['comment']) && ! is_string($options['comment'])) {
throw InvalidArgumentException::invalidType('"comment" option', $options['comment'], 'string');
}

if (isset($options['explain']) && ! is_bool($options['explain'])) {
throw InvalidArgumentException::invalidType('"explain" option', $options['explain'], 'boolean');
}
Expand Down
12 changes: 11 additions & 1 deletion src/Operation/Count.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ class Count implements Executable, Explainable
*
* * collation (document): Collation specification.
*
* * comment (mixed): Enables users to specify an arbitrary comment to help trace
* the operation through the database profiler, currentOp and logs. The
* default is to not send a value.
*
* The comment can be any valid BSON type for server versions 4.4 and above.
* Server versions between 3.6 and 4.2 only support string as comment,
* and providing a non-string type will result in a server-side error.
* Older server versions do not support comment for aggregate command at all,
* and providing one will result in a server-side error.
*
* * hint (string|document): The index to use. Specify either the index
* name as a string or the index key pattern as a document. If specified,
* then the query system will only consider plans using the hinted index.
Expand Down Expand Up @@ -195,7 +205,7 @@ private function createCommandDocument()
$cmd['hint'] = is_array($this->options['hint']) ? (object) $this->options['hint'] : $this->options['hint'];
}

foreach (['limit', 'maxTimeMS', 'skip'] as $option) {
foreach (['comment', 'limit', 'maxTimeMS', 'skip'] as $option) {
if (isset($this->options[$option])) {
$cmd[$option] = $this->options[$option];
}
Expand Down
12 changes: 11 additions & 1 deletion src/Operation/CountDocuments.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ class CountDocuments implements Executable
*
* * collation (document): Collation specification.
*
* * comment (mixed): Enables users to specify an arbitrary comment to help trace
* the operation through the database profiler, currentOp and logs. The
* default is to not send a value.
*
* The comment can be any valid BSON type for server versions 4.4 and above.
* Server versions between 3.6 and 4.2 only support string as comment,
* and providing a non-string type will result in a server-side error.
* Older server versions do not support comment for aggregate command at all,
* and providing one will result in a server-side error.
*
* * hint (string|document): The index to use. Specify either the index
* name as a string or the index key pattern as a document. If specified,
* then the query system will only consider plans using the hinted index.
Expand Down Expand Up @@ -107,7 +117,7 @@ public function __construct($databaseName, $collectionName, $filter, array $opti
$this->collectionName = (string) $collectionName;
$this->filter = $filter;

$this->aggregateOptions = array_intersect_key($options, ['collation' => 1, 'hint' => 1, 'maxTimeMS' => 1, 'readConcern' => 1, 'readPreference' => 1, 'session' => 1]);
$this->aggregateOptions = array_intersect_key($options, ['collation' => 1, 'comment' => 1, 'hint' => 1, 'maxTimeMS' => 1, 'readConcern' => 1, 'readPreference' => 1, 'session' => 1]);
$this->countOptions = array_intersect_key($options, ['limit' => 1, 'skip' => 1]);

$this->aggregate = $this->createAggregate();
Expand Down
20 changes: 13 additions & 7 deletions src/Operation/EstimatedDocumentCount.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

use function array_intersect_key;
use function is_integer;
use function MongoDB\server_supports_feature;

/**
* Operation for obtaining an estimated count of documents in a collection
Expand Down Expand Up @@ -61,6 +60,16 @@ class EstimatedDocumentCount implements Executable, Explainable
*
* Supported options:
*
* * comment (mixed): Enables users to specify an arbitrary comment to help trace
* the operation through the database profiler, currentOp and logs. The
* default is to not send a value.
*
* The comment can be any valid BSON type for server versions 4.4 and above.
* Server versions between 3.6 and 4.2 only support string as comment,
* and providing a non-string type will result in a server-side error.
* Older server versions do not support comment for aggregate command at all,
* and providing one will result in a server-side error.
*
* * maxTimeMS (integer): The maximum amount of time to allow the query to
* run.
*
Expand Down Expand Up @@ -96,7 +105,7 @@ public function __construct($databaseName, $collectionName, array $options = [])
throw InvalidArgumentException::invalidType('"session" option', $options['session'], Session::class);
}

$this->options = array_intersect_key($options, ['maxTimeMS' => 1, 'readConcern' => 1, 'readPreference' => 1, 'session' => 1]);
$this->options = array_intersect_key($options, ['comment' => 1, 'maxTimeMS' => 1, 'readConcern' => 1, 'readPreference' => 1, 'session' => 1]);
}

/**
Expand Down Expand Up @@ -157,12 +166,9 @@ private function createAggregate(): Aggregate
);
}

/** @return Aggregate|Count */
private function createCommand(Server $server)
private function createCommand(Server $server): Count
{
return server_supports_feature($server, self::$wireVersionForCollStats)
? $this->createAggregate()
: $this->createCount();
return $this->createCount();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, I believe createAggregate is no longer called and should be removed.

}

private function createCount(): Count
Expand Down
4 changes: 0 additions & 4 deletions tests/Operation/AggregateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ public function provideInvalidConstructorOptions()
$options[][] = ['collation' => $value];
}

foreach ($this->getInvalidStringValues() as $value) {
$options[][] = ['comment' => $value];
}

foreach ($this->getInvalidHintValues() as $value) {
$options[][] = ['hint' => $value];
}
Expand Down
Loading