Skip to content

PHPC-2518: Support sort option for updateOne and replaceOne #1794

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 1 commit into from
Feb 25, 2025
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
1 change: 1 addition & 0 deletions src/MongoDB/BulkWrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ static bool php_phongo_bulkwrite_update_apply_options(bson_t* boptions, zval* zo
PHONGO_BULKWRITE_APPEND_BOOL("upsert", upsert);
PHONGO_BULKWRITE_OPT_ARRAY("arrayFilters");
PHONGO_BULKWRITE_OPT_DOCUMENT("collation");
PHONGO_BULKWRITE_OPT_DOCUMENT("sort");

if (!php_phongo_bulkwrite_opt_hint(boptions, zoptions)) {
return false;
Expand Down
81 changes: 81 additions & 0 deletions tests/bulk/bulkwrite-update-008.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
--TEST--
MongoDB\Driver\BulkWrite::update() with sort option
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php skip_if_not_live(); ?>
<?php skip_if_server_version('<', '8.0'); ?>
<?php skip_if_not_clean(); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

class CommandLogger implements MongoDB\Driver\Monitoring\CommandSubscriber
{
public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event): void
{
if ($event->getCommandName() !== 'update') {
return;
}

printf("update included sort: %s\n", json_encode($event->getCommand()->updates[0]->sort));
}

public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event): void
{
}

public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event): void
{
}
}

$manager = create_test_manager();

$bulk = new MongoDB\Driver\BulkWrite();
$bulk->insert(['_id' => 1]);
$bulk->insert(['_id' => 2]);
$bulk->insert(['_id' => 3]);
$manager->executeBulkWrite(NS, $bulk);

MongoDB\Driver\Monitoring\addSubscriber(new CommandLogger);

$bulk = new MongoDB\Driver\BulkWrite;
$bulk->update(['_id' => ['$gt' => 1]], ['$set' => ['x' => 11]], ['sort' => ['_id' => 1]]);
$manager->executeBulkWrite(NS, $bulk);

$bulk = new MongoDB\Driver\BulkWrite;
$bulk->update(['_id' => ['$gt' => 1]], ['x' => 22], ['sort' => ['_id' => -1]]);
Copy link
Member

Choose a reason for hiding this comment

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

Did you want to test replaceOne here instead? This other method is in the spec commit. Otherwise I don't get the benefit of this 2nd update.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually testing the code path for mongoc_bulk_operation_replace_one_with_opts. PHPC's original bulk write API only provides a single BulkWrite::update() method, and we branch according to the structure of the second argument and multi option.

See: https://github.com/mongodb/mongo-php-driver/blob/1.20.1/src/MongoDB/BulkWrite.c#L452

This isn't relevant to the upcoming API for the bulkWrite command, which will have specific methods in PHPC.

$manager->executeBulkWrite(NS, $bulk);

$cursor = $manager->executeQuery(NS, new MongoDB\Driver\Query([]));

var_dump($cursor->toArray());

?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
update included sort: {"_id":1}
update included sort: {"_id":-1}
array(3) {
[0]=>
object(stdClass)#%d (%d) {
["_id"]=>
int(1)
}
[1]=>
object(stdClass)#%d (%d) {
["_id"]=>
int(2)
["x"]=>
int(11)
}
[2]=>
object(stdClass)#%d (%d) {
["_id"]=>
int(3)
["x"]=>
int(22)
}
}
===DONE===
22 changes: 22 additions & 0 deletions tests/bulk/bulkwrite-update_error-009.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
MongoDB\Driver\BulkWrite::update() with multi:true prohibits sort option
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

$manager = create_test_manager();
$bulk = new MongoDB\Driver\BulkWrite;

echo throws(function() use ($manager, $bulk) {
$bulk->update(['x' => ['$gt' => 1]], ['$set' => ['y' => 11]], ['multi' => true, 'sort' => ['x' => 1]]);
}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n";

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Invalid option 'sort'
===DONE===
Loading