Skip to content

PHPLIB-539: Allow hinting for delete command #732

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 6 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 7 additions & 4 deletions .travis/install-extension.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ tpecl () {
fi
}

if [ "x${DRIVER_BRANCH}" != "x" ]; then
echo "Compiling driver branch ${DRIVER_BRANCH}"
if [ "x${DRIVER_BRANCH}" != "x" ] || [ "x${DRIVER_REPO}" != "x" ]; then
CLONE_REPO=${DRIVER_REPO:-https://github.com/mongodb/mongo-php-driver}
CHECKOUT_BRANCH=${DRIVER_BRANCH:-master}

echo "Compiling driver branch ${CHECKOUT_BRANCH} from repository ${CLONE_REPO}"

mkdir -p /tmp/compile
git clone https://github.com/mongodb/mongo-php-driver /tmp/compile/mongo-php-driver
git clone ${CLONE_REPO} /tmp/compile/mongo-php-driver
cd /tmp/compile/mongo-php-driver

git checkout ${DRIVER_BRANCH}
git checkout ${CHECKOUT_BRANCH}
git submodule update --init
phpize
./configure --enable-mongodb-developer-flags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ source:
file: apiargs-MongoDBCollection-common-option.yaml
ref: collation
---
source:
file: apiargs-common-option.yaml
ref: hint
post: |
This option is available in MongoDB 4.4+ and will result in an exception at
execution time if specified for an older server version.

.. versionadded:: 1.7
---
source:
file: apiargs-common-option.yaml
ref: session
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ source:
file: apiargs-MongoDBCollection-common-option.yaml
ref: collation
---
source:
file: apiargs-common-option.yaml
ref: hint
post: |
This option is available in MongoDB 4.4+ and will result in an exception at
execution time if specified for an older server version.

.. versionadded:: 1.7
---
source:
file: apiargs-common-option.yaml
ref: session
Expand Down
26 changes: 26 additions & 0 deletions src/Operation/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use MongoDB\Exception\UnsupportedException;
use function is_array;
use function is_object;
use function is_string;
use function MongoDB\server_supports_feature;

/**
Expand All @@ -43,6 +44,9 @@ class Delete implements Executable, Explainable
/** @var integer */
private static $wireVersionForCollation = 5;

/** @var int */
private static $wireVersionForHintServerSideError = 5;

/** @var string */
private $databaseName;

Expand All @@ -68,6 +72,13 @@ class Delete implements Executable, Explainable
* This is not supported for server versions < 3.4 and will result in an
* exception at execution time if used.
*
* * 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.
*
* This is not supported for server versions < 4.4 and will result in an
* exception at execution time if used.
*
* * session (MongoDB\Driver\Session): Client session.
*
* Sessions are not supported for server versions < 3.6.
Expand Down Expand Up @@ -97,6 +108,10 @@ public function __construct($databaseName, $collectionName, $filter, $limit, arr
throw InvalidArgumentException::invalidType('"collation" option', $options['collation'], 'array or object');
}

if (isset($options['hint']) && ! is_string($options['hint']) && ! is_array($options['hint']) && ! is_object($options['hint'])) {
throw InvalidArgumentException::invalidType('"hint" option', $options['hint'], ['string', 'array', 'object']);
}

if (isset($options['session']) && ! $options['session'] instanceof Session) {
throw InvalidArgumentException::invalidType('"session" option', $options['session'], Session::class);
}
Expand Down Expand Up @@ -130,6 +145,13 @@ public function execute(Server $server)
throw UnsupportedException::collationNotSupported();
}

/* Server versions >= 3.4.0 raise errors for unknown update
* options. For previous versions, the CRUD spec requires a client-side
* error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
throw UnsupportedException::hintNotSupported();
}

$inTransaction = isset($this->options['session']) && $this->options['session']->isInTransaction();
if ($inTransaction && isset($this->options['writeConcern'])) {
throw UnsupportedException::writeConcernNotSupportedInTransaction();
Expand Down Expand Up @@ -170,6 +192,10 @@ private function createDeleteOptions()
$deleteOptions['collation'] = (object) $this->options['collation'];
}

if (isset($this->options['hint'])) {
$deleteOptions['hint'] = $this->options['hint'];
}

return $deleteOptions;
}

Expand Down
7 changes: 7 additions & 0 deletions src/Operation/DeleteMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ class DeleteMany implements Executable, Explainable
* This is not supported for server versions < 3.4 and will result in an
* exception at execution time if used.
*
* * 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.
*
* This is not supported for server versions < 4.4 and will result in an
* exception at execution time if used.
*
* * session (MongoDB\Driver\Session): Client session.
*
* Sessions are not supported for server versions < 3.6.
Expand Down
7 changes: 7 additions & 0 deletions src/Operation/DeleteOne.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ class DeleteOne implements Executable, Explainable
* This is not supported for server versions < 3.4 and will result in an
* exception at execution time if used.
*
* * 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.
*
* This is not supported for server versions < 4.4 and will result in an
* exception at execution time if used.
*
* * session (MongoDB\Driver\Session): Client session.
*
* Sessions are not supported for server versions < 3.6.
Expand Down
24 changes: 21 additions & 3 deletions src/Operation/FindAndModify.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class FindAndModify implements Executable, Explainable
/** @var integer */
private static $wireVersionForDocumentLevelValidation = 4;

/** @var integer */
private static $wireVersionForHint = 9;

/** @var integer */
private static $wireVersionForHintServerSideError = 8;

Expand Down Expand Up @@ -99,8 +102,7 @@ class FindAndModify implements Executable, Explainable
* 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.
*
* This is only supported for update and replace operations (i.e. remove
* option is false) on server versions >= 4.4. Using this option in
* This is only supported on server versions >= 4.4. Using this option in
* other contexts will result in an exception at execution time.
*
* * maxTimeMS (integer): The maximum amount of time to allow the query to
Expand Down Expand Up @@ -246,7 +248,7 @@ public function execute(Server $server)
* options (SERVER-40005), but the CRUD spec requires client-side errors
* for server versions < 4.2. For later versions, we'll rely on the
* server to either utilize the option or report its own error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
if (isset($this->options['hint']) && ! $this->isHintSupported($server)) {
throw UnsupportedException::hintNotSupported();
}

Expand Down Expand Up @@ -339,4 +341,20 @@ private function createOptions()

return $options;
}

private function isAcknowledgedWriteConcern() : bool
{
if (! isset($this->options['writeConcern'])) {
return true;
}

return $this->options['writeConcern']->getW() > 1 || $this->options['writeConcern']->getJournal();
}

private function isHintSupported(Server $server) : bool
{
$requiredWireVersion = $this->isAcknowledgedWriteConcern() ? self::$wireVersionForHintServerSideError : self::$wireVersionForHint;

return server_supports_feature($server, $requiredWireVersion);
}
}
7 changes: 7 additions & 0 deletions src/Operation/FindOneAndDelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ class FindOneAndDelete implements Executable, Explainable
* This is not supported for server versions < 3.4 and will result in an
* exception at execution time if used.
*
* * 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.
*
* This is not supported for server versions < 4.4 and will result in an
* exception at execution time if used.
*
* * maxTimeMS (integer): The maximum amount of time to allow the query to
* run.
*
Expand Down
7 changes: 5 additions & 2 deletions src/Operation/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Update implements Executable, Explainable
private static $wireVersionForDocumentLevelValidation = 4;

/** @var integer */
private static $wireVersionForHint = 8;
private static $wireVersionForHintServerSideError = 5;

/** @var string */
private $databaseName;
Expand Down Expand Up @@ -202,7 +202,10 @@ public function execute(Server $server)
throw UnsupportedException::collationNotSupported();
}

if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)) {
/* Server versions >= 3.4.0 raise errors for unknown update
* options. For previous versions, the CRUD spec requires a client-side
* error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
throw UnsupportedException::hintNotSupported();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/SpecTests/CrudSpecTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function testCrud(stdClass $test, array $runOn = null, array $data, $data
}

if (isset($test->expectations)) {
$commandExpectations = CommandExpectations::fromCrud($test->expectations);
$commandExpectations = CommandExpectations::fromCrud((array) $test->expectations);
$commandExpectations->startMonitoring();
}

Expand Down
150 changes: 150 additions & 0 deletions tests/SpecTests/crud/bulkWrite-delete-hint-clientError.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
{
"runOn": [
{
"maxServerVersion": "3.3.99"
}
],
"data": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
},
{
"_id": 3,
"x": 33
},
{
"_id": 4,
"x": 44
}
],
"collection_name": "BulkWrite_delete_hint",
"tests": [
{
"description": "BulkWrite deleteOne with hints unsupported (client-side error)",
"operations": [
{
"name": "bulkWrite",
"arguments": {
"requests": [
{
"name": "deleteOne",
"arguments": {
"filter": {
"_id": 1
},
"hint": "_id_"
}
},
{
"name": "deleteOne",
"arguments": {
"filter": {
"_id": 2
},
"hint": {
"_id": 1
}
}
}
],
"options": {
"ordered": true
}
},
"error": true
}
],
"expectations": [],
"outcome": {
"collection": {
"data": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
},
{
"_id": 3,
"x": 33
},
{
"_id": 4,
"x": 44
}
]
}
}
},
{
"description": "BulkWrite deleteMany with hints unsupported (client-side error)",
"operations": [
{
"name": "bulkWrite",
"arguments": {
"requests": [
{
"name": "deleteMany",
"arguments": {
"filter": {
"_id": {
"$lt": 3
}
},
"hint": "_id_"
}
},
{
"name": "deleteMany",
"arguments": {
"filter": {
"_id": {
"$gte": 4
}
},
"hint": {
"_id": 1
}
}
}
],
"options": {
"ordered": true
}
},
"error": true
}
],
"expectations": [],
"outcome": {
"collection": {
"data": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
},
{
"_id": 3,
"x": 33
},
{
"_id": 4,
"x": 44
}
]
}
}
}
]
}
Loading