Skip to content

Commit d6ff77f

Browse files
abnegateclaude
andcommitted
fix(mariadb): surface the error of a later statement in a multi-statement string
MySQL and MariaDB run each statement of a ';'-joined string, but execute() reports only the first one's error; a later statement's error waits in its result. The adapter joins statements this way for a relationship's columns and a collection's permissions table, so a two-column relationship rename whose second ALTER failed reported success and stored the new keys over a column that was never renamed. executeStatement() now reaches every result, so such a failure fails the call. PostgreSQL already failed the call, and SQLite runs its statements one by one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent cbb4345 commit d6ff77f

2 files changed

Lines changed: 73 additions & 0 deletions

File tree

‎src/Database/Adapter/MariaDB.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,23 @@ protected function decodePolygon(string $wkb): array
10551055
*/
10561056
private int $appliedRound = 0;
10571057

1058+
/**
1059+
* MySQL and MariaDB run every statement of a multi-statement string, but report only the first one's error from
1060+
* execute(); a later statement's error surfaces when its result is reached. Each result is reached here, so a
1061+
* statement that fails after another succeeded fails the call instead of being lost.
1062+
*/
1063+
#[\Override]
1064+
protected function executeStatement(string $sql, Event $event): bool
1065+
{
1066+
$statement = $this->prepareStatement($sql, $event);
1067+
$executed = $this->execute($statement);
1068+
1069+
while ($statement->nextRowset()) {
1070+
}
1071+
1072+
return $executed;
1073+
}
1074+
10581075
/**
10591076
* @param PDOStatement|DatabasePDOStatement|PDOStatementProxy $statement
10601077
*/

‎tests/e2e/Adapter/Scopes/SchemaReconciliationTests.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Tests\E2E\Adapter\Scopes;
44

55
use Throwable;
6+
use Utopia\Database\Adapter\Feature;
67
use Utopia\Database\Attribute;
78
use Utopia\Database\Capability;
89
use Utopia\Database\Collection;
@@ -11,6 +12,8 @@
1112
use Utopia\Database\Exception\Duplicate as DuplicateException;
1213
use Utopia\Database\Index;
1314
use Utopia\Database\Permission;
15+
use Utopia\Database\Relationship;
16+
use Utopia\Database\RelationshipUpdate;
1417
use Utopia\Database\Role;
1518
use Utopia\Database\Schema\Index as SchemaIndex;
1619
use Utopia\Database\Storage;
@@ -94,6 +97,59 @@ public function testAnOrphanColumnOfTheRequestedTypeIsReused(): void
9497
}
9598
}
9699

100+
public function testARelationshipRenameThatFailsPartWayConvergesOnRetry(): void
101+
{
102+
$database = $this->getDatabase();
103+
$adapter = $database->getAdapter();
104+
105+
if (! $adapter->supports(Capability::SchemaIntrospection) || ! $adapter->hasFeature(Feature\Relationships::class)) {
106+
$this->expectNotToPerformAssertions();
107+
108+
return;
109+
}
110+
111+
$books = 'halfRenamedBooks';
112+
$authors = 'halfRenamedAuthors';
113+
foreach ([$books, $authors] as $collection) {
114+
$database->createCollection(Collection::create(id: $collection, attributes: [
115+
Attribute::string(key: 'name', size: 64),
116+
], permissions: [
117+
Permission::create(Role::any()),
118+
Permission::read(Role::any()),
119+
Permission::update(Role::any()),
120+
Permission::delete(Role::any()),
121+
], documentSecurity: false));
122+
}
123+
124+
try {
125+
$database->createRelationship($books, Relationship::oneToOne(relatedCollection: $authors, twoWay: true, key: 'author', twoWayKey: 'book'));
126+
$database->createDocument($authors, new Document([Document::ID => 'herbert', 'name' => 'Herbert']));
127+
$database->createDocument($books, new Document([Document::ID => 'dune', 'name' => 'Dune', 'author' => 'herbert']));
128+
$adapter->createAttribute($authors, Attribute::string(key: 'novel', size: 64));
129+
$update = new RelationshipUpdate(key: 'writer', twoWayKey: 'novel');
130+
131+
try {
132+
$database->updateRelationship($books, 'author', $update);
133+
$this->fail('A rename whose second column the engine refuses must fail');
134+
} catch (Throwable $error) {
135+
$this->assertNotInstanceOf(\PHPUnit\Framework\AssertionFailedError::class, $error);
136+
}
137+
138+
$adapter->deleteAttribute($authors, 'novel');
139+
$database->updateRelationship($books, 'author', $update);
140+
141+
$writer = $database->getDocument($books, 'dune')->getAttribute('writer');
142+
$this->assertInstanceOf(Document::class, $writer);
143+
$this->assertSame('herbert', $writer->getId());
144+
$novel = $database->getDocument($authors, 'herbert')->getAttribute('novel');
145+
$this->assertInstanceOf(Document::class, $novel);
146+
$this->assertSame('dune', $novel->getId());
147+
} finally {
148+
$database->deleteCollection($books);
149+
$database->deleteCollection($authors);
150+
}
151+
}
152+
97153
public function testAnOrphanColumnOfAnotherTypeIsReplacedOrRefusedUnderSharedTables(): void
98154
{
99155
$database = $this->getDatabase();

0 commit comments

Comments
 (0)