Skip to content

Partial update #886

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.

Sign up for GitHub

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

Open
fogelito wants to merge 19 commits into
base: main
Choose a base branch
from
+28 −19
Open

Partial update #886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0e6ad57
partialUpdate
fogelito Jun 9, 2026
90e3634
update
fogelito Jun 10, 2026
4e56486
add collection
fogelito Jun 10, 2026
3d91101
Merge branch 'main' of github.com:utopia-php/database into partial-up…
fogelito Jun 10, 2026
8508d63
fix validation
fogelito Jun 10, 2026
6b2a7b5
remove unset
fogelito Jun 10, 2026
6d4db8d
fix perms
fogelito Jun 10, 2026
e29ee24
fix createdAt
fogelito Jun 10, 2026
f94cc01
(fix): CI — propagate partial-update semantics to SQLite/Postgres ada…
github-actions[bot] Jun 10, 2026
3a0c449
use clone
fogelito Jun 10, 2026
a879acd
Merge remote-tracking branch 'origin/partial-update' into partial-update
fogelito Jun 10, 2026
cec6e43
fix perms
fogelito Jun 10, 2026
a50e28f
Merge branch 'main' of github.com:utopia-php/database into partial-up…
fogelito Jun 10, 2026
76b2374
run
fogelito Jun 10, 2026
d14540d
revert
fogelito Jun 10, 2026
40d4b5c
revert
fogelito Jun 10, 2026
93e0eef
remove clone
fogelito Jun 10, 2026
5f0caca
Pstgres
fogelito Jun 10, 2026
5cda737
Pstgres
fogelito Jun 10, 2026
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
26 changes: 18 additions & 8 deletions src/Database/Adapter/MariaDB.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,9 +977,18 @@ public function updateDocument(Document $collection, string $id, Document $docum
$spatialAttributes = $this->getSpatialAttributes($collection);
$collection = $collection->getId();
$attributes = $document->getAttributes();
$attributes['_createdAt'] = $document->getCreatedAt();
$attributes['_updatedAt'] = $document->getUpdatedAt();
$attributes['_permissions'] = json_encode($document->getPermissions());
if ($document->offsetExists('$updatedAt')) {
$attributes['_updatedAt'] = $document->getUpdatedAt();
}
if ($document->offsetExists('$id')) {
$attributes['_uid'] = $document->getId();
}
if ($document->offsetExists('$createdAt')) {
$attributes['_createdAt'] = $document->getCreatedAt();
}
if ($document->offsetExists('$permissions')) {
$attributes['_permissions'] = json_encode($document->getPermissions());
}

$name = $this->filter($collection);
$columns = '';
Expand All @@ -998,7 +1007,8 @@ public function updateDocument(Document $collection, string $id, Document $docum
* Get current permissions from the database
*/
$sqlPermissions = $this->getPDO()->prepare($sql);
$sqlPermissions->bindValue(':_uid', $document->getId());

$sqlPermissions->bindValue(':_uid', $id);

if ($this->sharedTables) {
$sqlPermissions->bindValue(':_tenant', $this->tenant);
Expand Down Expand Up @@ -1071,7 +1081,7 @@ public function updateDocument(Document $collection, string $id, Document $docum
$removeQuery = $this->trigger(Database::EVENT_PERMISSIONS_DELETE, $removeQuery);

$stmtRemovePermissions = $this->getPDO()->prepare($removeQuery);
$stmtRemovePermissions->bindValue(':_uid', $document->getId());
$stmtRemovePermissions->bindValue(':_uid', $id);

if ($this->sharedTables) {
$stmtRemovePermissions->bindValue(':_tenant', $this->tenant);
Expand Down Expand Up @@ -1119,7 +1129,8 @@ public function updateDocument(Document $collection, string $id, Document $docum

$stmtAddPermissions = $this->getPDO()->prepare($sql);

$stmtAddPermissions->bindValue(":_uid", $document->getId());
$newUid = $document->offsetExists('$id') ? $document->getId() : $id;
$stmtAddPermissions->bindValue(":_uid", $newUid);

if ($this->sharedTables) {
$stmtAddPermissions->bindValue(":_tenant", $this->tenant);
Expand Down Expand Up @@ -1168,7 +1179,7 @@ public function updateDocument(Document $collection, string $id, Document $docum

$sql = "
UPDATE {$this->getSQLTable($name)}
SET {$columns} _uid = :_newUid
SET " . \rtrim($columns, ',') . "
WHERE _id=:_sequence
{$this->getTenantQuery($collection)}
";
Comment on lines 1180 to 1185

@greptile-apps greptile-apps Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Empty SET clause produces invalid SQL for no-op partial updates

Before this PR the SET clause always had at least _uid = :_newUid as a guaranteed trailing column. That fallback is now removed. When the partial document carries no regular attributes (e.g. $permissions-only) and $shouldUpdate is false (so $updatedAt is not injected), $attributes is empty, $columns stays '', and \rtrim('', ',') returns ''. The resulting SQL — UPDATE … SET WHERE _id=:_sequence — is a syntax error that causes a PDO exception.

A concrete failing path: updateDocument($col, $id, new Document(['$permissions' => $exactSamePerms])). No attribute changes → $shouldUpdate = false → no $updatedAt attribute set. No $id in the document → no _uid in attributes. Permissions unchanged → $skipPermissionsUpdate = true. Adapter called with empty $attributes → $columns = '' → invalid SQL.

The same gap exists in Postgres.php and SQLite.php.

Comment on lines 1180 to 1185

@greptile-apps greptile-apps Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Empty SET clause on no-op partial updates

Before this PR, SET {$columns} _uid = :_newUid guaranteed a non-empty SET clause because _uid was always appended. That unconditional trailing column is now removed. When the adapter is called with a document that carries no tracked attributes — e.g., a caller passes new Document([]) or a document whose only keys are internal attributes not handled by the adapter's offsetExists checks — $attributes is empty, $columns stays '', \rtrim('', ',') returns '', and the resulting SQL is UPDATE … SET WHERE _id=:_sequence, which is a PDO syntax error.

One concrete path: updateDocument is always called regardless of $shouldUpdate; when $shouldUpdate is false the adapter receives no _updatedAt, and if the partial document also has no user attributes, no $id, no $createdAt, and no $permissions, $attributes is empty. The same gap exists in Postgres.php and SQLite.php.

A guard before the prepare call that skips the UPDATE (or emits a trivial SET _id=_id) when $columns is empty would prevent this.

Expand All @@ -1178,7 +1189,6 @@ public function updateDocument(Document $collection, string $id, Document $docum
$stmt = $this->getPDO()->prepare($sql);

$stmt->bindValue(':_sequence', $document->getSequence());
$stmt->bindValue(':_newUid', $document->getId());

if ($this->sharedTables) {
$stmt->bindValue(':_tenant', $this->tenant);
Expand Down
21 changes: 10 additions & 11 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6158,18 +6158,13 @@ public function updateDocument(string $collection, string $id, Document $documen

$skipPermissionsUpdate = ($originalPermissions === $currentPermissions);
}
$createdAt = $document->getCreatedAt();

$document = \array_merge($old->getArrayCopy(), $document->getArrayCopy());
$document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collection ID
$document['$createdAt'] = ($createdAt === null || !$this->preserveDates) ? $old->getCreatedAt() : $createdAt;
if (!$this->preserveDates || $document->getCreatedAt() === null) {
$document->removeAttribute('$createdAt');
}

if ($this->adapter->getSharedTables()) {
$tenant = $old->getTenant();
$document['$tenant'] = $tenant;
$old->setAttribute('$tenant', $tenant); // Normalize for strict comparison
$old->setAttribute('$tenant', $old->getTenant()); // Normalize for strict comparison
}
$document = new Document($document);

$attributes = $collection->getAttribute('attributes', []);

Expand Down Expand Up @@ -6311,10 +6306,10 @@ public function updateDocument(string $collection, string $id, Document $documen
throw new ConflictException('Document was updated after the request timestamp');
}

$document = $this->encode($collection, $document);
$document = $this->encode($collection, clone $document, applyDefaults: false);

if ($this->validate) {
$structureValidator = new Structure(
$structureValidator = new PartialStructure(
$collection,
$this->adapter->getIdAttributeType(),
$this->adapter->getMinDateTime(),
Expand All @@ -6334,8 +6329,12 @@ public function updateDocument(string $collection, string $id, Document $documen

$document = $this->adapter->castingBefore($collection, $document);

$document->setAttribute('$sequence', $old->getSequence());

$this->adapter->updateDocument($collection, $id, $document, $skipPermissionsUpdate);

$document = new Document(\array_merge($old->getArrayCopy(), $document->getArrayCopy()));

$document = $this->adapter->castingAfter($collection, $document);

$this->purgeCachedDocument($collection->getId(), $id);
Expand Down
Loading