Partial update

[fogelito]

Summary by CodeRabbit

• Bug Fixes

  • Prevents accidental overwriting of internal metadata during updates.
  • Honors configuration for preserving or omitting creation timestamps.
  • Reduces inconsistent tenant/version handling to avoid conflicting saves.

• Improvements

  • Conditional handling of metadata and permissions to avoid unintended changes.
  • Safer merging and stricter validation of persisted vs incoming data for more reliable updates.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Database::updateDocument sanitizes in-flight updates (removes internals, optional $createdAt, tenant normalization), validates with PartialStructure against persisted document, restores persisted $sequence before adapter call, and merges persisted data with adapter-updated attributes. MariaDB/Postgres/SQLite adapters now conditionally map internal fields, adjust permission id bindings, and trim UPDATE SET/bindings.

Changes

Document update sanitization and adapter metadata updates

Layer / File(s)	Summary
Database sanitization, normalization, validation, sequence restore, and merge src/Database/Database.php	Strip internal fields from incoming documents; optionally remove $createdAt when preserveDates is false; normalize $tenant in shared-tables mode; validate updates with PartialStructure using the persisted $old as currentDocument; restore persisted $sequence onto the in-flight document before calling the adapter; after adapter returns, merge the persisted array with adapter-updated attributes and continue existing casting/cache/post-update logic.
MariaDB conditional metadata copy and permission id binding / UPDATE clause src/Database/Adapter/MariaDB.php	updateDocument() conditionally copies _updatedAt, _uid, _createdAt, and _permissions only when those offsets exist on the incoming Document; permission read/delete bind :_uid from method $id; inserted permission _uid uses computed newUid (incoming $id if present else method $id); UPDATE SET uses trimmed $columns and the :_newUid binding was removed.
Postgres conditional metadata copy and permission id binding / UPDATE clause src/Database/Adapter/Postgres.php	updateDocument() conditionally maps _updatedAt, _uid, _createdAt, _permissions only when present on the incoming Document; permission queries/deletes bind :_uid from method $id; inserted permissions compute _uid from incoming $id when present; UPDATE SET no longer appends _uid = :_newUid and :_newUid binding removed.
SQLite conditional metadata copy and permission id binding / UPDATE clause src/Database/Adapter/SQLite.php	updateDocument() conditionally sets _updatedAt, _uid, _createdAt, _permissions only when the incoming Document exposes them; permission reads/deletes bind :_uid from method $id; inserting permissions computes newUid from incoming $id if present; UPDATE SET stops appending _uid = :_newUid and its binding removed.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• utopia-php/database#611: Related changes to permissions handling during updateDocument (permission sync behavior).
• utopia-php/database#642: Overlapping edits to createdAt/updatedAt handling and preserveDates behavior in update flows.
• utopia-php/database#631: Related permission synchronization adjustments in updateDocument.

Suggested reviewers

• abnegate

Poem

🐰 I nibble keys both old and new,
I drop the dates that shouldn't stew,
Sequence snug where it must stay,
Adapters mind which fields to convey,
The merged doc hops home, all true.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Partial update' is vague and does not clearly convey what specific changes are being made in this pull request.	Clarify the title to better describe the main change, such as 'Support conditional metadata updates in document operations' or 'Make metadata updates conditional on document field presence'.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch partial-update

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR converts updateDocument from a full-document-replace pattern to a true partial update: only the fields present in the caller's Document are written to the database, guarded by a PartialStructure validator and conditional offsetExists checks in the adapter layer. Permissions and timestamp fields are now written only when explicitly supplied.

• Database.php removes the pre-adapter full-merge with $old, instead deferring it to after the adapter call so only the partial set of fields reaches the SQL layer; $sequence is injected from $old after castingBefore.
• MariaDB.php gates each internal column (_updatedAt, _uid, _createdAt, _permissions) behind offsetExists checks and fixes permissions queries to bind the $id parameter instead of $document->getId() (which could be null for a partial document).

Confidence Score: 3/5

The adapter can generate syntactically invalid SQL when a partial document carries no tracked columns, which would throw a PDO exception on any such call.

The removal of the unconditional _uid = :_newUid trailing column means the SET clause can be empty for certain partial documents (no user attributes, no $updatedAt, no $id, no $permissions). This SQL syntax error is a runtime defect on a real code path. The issue also affects the parallel Postgres and SQLite adapters that are not in this diff. Combined with the breadth of the change (full update path refactored), caution is warranted before merging.

The empty-SET-clause defect lives in src/Database/Adapter/MariaDB.php (and its siblings Postgres.php/SQLite.php outside this diff).

Important Files Changed

Filename	Overview
src/Database/Adapter/MariaDB.php	Partial-update refactor: permissions binding now correctly uses $id parameter, but the unconditional _uid = :_newUid trailing column removed from the SET clause leaves a path to empty SET → invalid SQL when no tracked attributes are present.
src/Database/Database.php	Switches to true partial-update semantics: only provided fields are merged/encoded, PartialStructure validator is used, $sequence is injected after castingBefore, and $old is merged back post-adapter-call. The $collection switchability guard removed from the old merge has no DB impact since that column is not updated.

_{Reviews (12): Last reviewed commit: "Pstgres" | Re-trigger Greptile}

[claude]

CI failure on run 27276370273

All adapter test jobs failed (MongoDB, Postgres, MariaDB, SharedTables/*, Pool, Mirror, Memory, …) with the same family of errors triggered by the new partial-update path in updateDocument:

• Invalid document structure: Attribute "$createdAt" has invalid type
• Invalid relationship value. Must be either an array of documents or document IDs, NULL given.
• Call to a member function getId() on null (reverse relationship lost)
• Date format mismatch: '2026-06-10T12:32:53.036+00:00' vs '1781094773036'

Root cause

The previous implementation did $document = new Document(\array_merge($old, $document)) early in updateDocument. That single line was load-bearing for four reasons. Removing it broke each one:

1. Caller's document was being mutated. encode() and castingBefore() rewrite attributes in place. Without the early new Document(...), the caller's reference is mutated — so on a second call the same document goes in already cast to BSON/native format, and structure validation rejects it (e.g. date field becomes invalid; $updatedAt reads back as raw milliseconds).
2. Nested associative arrays were no longer wrapped. Document's constructor recursively converts inner arrays containing $id/$collection into Documents. Without that, updateDocumentRelationships saw plain arrays and threw Must be either a document or document ID.
3. updateDocumentRelationships walked every relationship on the collection — even those absent from the partial document. For a partial update that omitted a relationship key, $value was null and the code either threw NULL given or, worse, took the case 'NULL' branch and nulled out the inverse two-way key, breaking the reverse link.
4. $createdAt = null was no longer falling back to $old->getCreatedAt() when preserveDates was true.

Fix (commit)

src/Database/Database.php:

• Clone the input at the top of updateDocument via new Document($document->getArrayCopy()). Restores caller-isolation and nested-Document wrapping in one line.
• Drop $createdAt from the partial document when the caller passed null OR preserveDates is disabled, so the final array_merge($old, $document) reinstates the original value.
• Skip relationships not present in the partial document inside updateDocumentRelationships (offsetExists($key) guard) — partial updates must leave omitted links untouched.
• Pass applyDefaults: false to encode() so partial updates don't silently fill in defaults for attributes the caller omitted.

[github-actions]

Claude could not apply patches from: healing (merge conflicts with current branch tip). These tasks need manual attention.

[claude]

CI healing — SQLite/Postgres partial-update propagation

What failed
Run 27276801963 failed across every adapter. The SQLite SharedTables job (in /tmp/ci-failure-logs.txt) showed ~60 failures dominated by operator tests like:

• testOperatorIncrementExceedsMaxValue — Failed asserting that null matches expected 100
• testOperatorMultiply/Divide/PowerWithNegative… — Utopia\Database\Operator Object (…) does not match expected type "double"

…meaning a partial update with Operator::increment(...) left the row in a state where subsequent getDocument() returned null (the assertion saw the un-applied Operator object, not the computed value).

Root cause
Database::updateDocument was refactored so that internal attributes ($createdAt, $updatedAt, $permissions, $id) are only present on the $document when the caller explicitly provided them. MariaDB::updateDocument was updated to honor that via offsetExists checks, but SQLite and Postgres still unconditionally wrote those columns:

$attributes['_createdAt']   = $document->getCreatedAt();   // null when not provided
$attributes['_updatedAt']   = $document->getUpdatedAt();
$attributes['_permissions'] = json_encode($document->getPermissions()); // "[]"
…
SET {$columns}, _uid = :_newUid
$stmt->bindValue(':_newUid', $document->getId()); // empty string for partial updates

So a partial updateDocument(coll, id, new Document(['score' => Operator::increment(5)])) rewrote _uid to ''. The increment SQL ran, but the row's identity was destroyed, and getDocument($id) could no longer find it → null score → cascade of operator-test failures.

Additionally, MariaDB::updateDocument was unconditionally setting _updatedAt even when Database::updateDocument had decided shouldUpdate=false (and therefore not set $updatedAt on the document), so relationship-only updates were stamping the parent row's _updatedAt to NULL — visible in testNoInvalidKeysWithRelationships ('$updatedAt' => null actual vs ISO timestamp expected).

What I fixed (in commit 7f2b75fd)

• src/Database/Adapter/SQLite.php: mirror MariaDB's offsetExists('$updatedAt'|'$id'|'$createdAt'|'$permissions') guards; bind :_uid for permission queries against the caller-supplied $id (not $document->getId()); drop the always-present , _uid = :_newUid clause (UID changes now flow through the conditional _uid attribute).
• src/Database/Adapter/Postgres.php: same set of changes.
• src/Database/Adapter/MariaDB.php: _updatedAt now only written when the document has $updatedAt (already true for _createdAt/_permissions/_uid).

Scope note
This addresses the SQLite/Postgres parity gap and the _updatedAt/NULL relationship issue. There are still failures in MariaDB visible in the same run that look like deeper partial-update semantic gaps — testSingleDocumentDateOperations (PartialStructure rejects explicit $createdAt = null), several relationship tests passing arrays into the new "is value a string/Document/null?" comparison loop, testNoChangeUpdateDocumentWithoutPermission (date format leaking through castingBefore into the merged return), and testSpatialArrayWKTConversionInUpdateDocument. Those are partial-update feature bugs, not adapter-parity issues, and I'm leaving them for the PR author since fixing them touches the core merge/encode/validation flow rather than the adapter layer.

[claude]

CI fix — partial updateDocument was corrupting untouched fields

What failed (CI run 27275419193): every adapter job failed with a mix of relationship exceptions, operator math errors (e.g., 2.5 != 18.0, '-middle' vs 'start-middle'), spatial decode TypeErrors, and $updatedAt going null. Root cause was the new partial-update path corrupting attributes that the caller didn't include in the update.

What I fixed:

1. Stop mutating the caller's Document. updateDocument now clones with new Document($document->getArrayCopy()) before any work. This isolates the encode-in-place from the caller and also normalizes nested arrays with $id/$collection into Document instances — that fixes the array_map → Invalid relationship value. Must be either a document or document ID. paths.

2. Don't apply defaults during partial encode. Pass applyDefaults: false so an operator-only update doesn't reset every other column with a default to its default value (was the cause of 2.5 != 18.0 and '-middle' instead of 'start-middle').

3. Skip relationships not present in the partial. updateDocumentRelationships was iterating all collection relationships and treating absent → null as "clear this relationship", wiping bidirectional links. Now keys not in $document are left alone.

4. Encode $old before merging. After the adapter update we merged decoded $old with the encoded partial, then ran the outer decode, which TypeError'd on already-decoded spatial arrays. Encoding $old first (idempotent on encoded values) keeps the merged document uniformly encoded so the outer decode runs cleanly.

5. Make MariaDB/MySQL/Postgres/SQLite adapters honor the partial. They unconditionally wrote _createdAt/_updatedAt/_permissions/_uid on every UPDATE — nulling out timestamps and permissions when the caller didn't include them. Each column is now guarded by offsetExists and the UPDATE is skipped when no columns remain.

Redis and MongoDB adapters already merge or $set partially, so they didn't need changes.

[github-actions]

Claude pushed fixes from: healing

e29ee24...f94cc01

[github-actions]

Claude could not apply patches from: healing (merge conflicts with current branch tip). These tasks need manual attention.

[claude]

CI run 27277729469 failed at the Set up Docker Buildx step with a Docker Hub network timeout:

#1 pulling image moby/buildkit:buildx-stable-1
ERROR: Error response from daemon: Get "https://registry-1.docker.io/v2/": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)

This is a flaky infrastructure failure — the runner couldn't reach registry-1.docker.io to pull the BuildKit image. It happened before any project code was built or tested, so it's unrelated to this PR. No code changes are warranted; a re-run should pass.

[claude]

Healing run on CI failure 27277229719

What failed: the partial-update refactor was incomplete. Two regressions caused the bulk of the cross-adapter errors (12 errors on MongoDB, 13 on MariaDB/MySQL, 26 on Postgres, plus shared-tables variants):

1. updateDocumentRelationships (Database.php) iterated over collection-defined relationships and read $document->getAttribute($key) regardless of whether the partial document carried that key. When the key was absent the value came back NULL — for to-one relationships this fell into the explicit-disconnect branch and unlinked the existing relation, for to-many it threw Invalid relationship value … NULL given.
2. Postgres adapter updateDocument was never updated to match the MariaDB partial-update changes — it still added _createdAt = $document->getCreatedAt(), _permissions = json_encode(...), and kept the trailing _uid = :_newUid in the SET clause. With a partial document this set NOT NULL columns to NULL and made the row unfindable (hence the downstream Document not found in testCreateUpdateBigIntAndIncrementDecrement).

What I changed:

• src/Database/Database.php: in updateDocumentRelationships, continue when !$document->offsetExists($key) so the existing relation is preserved. Explicit NULL still flows through and disconnects.
• src/Database/Adapter/Postgres.php: mirror the MariaDB adapter — only set _uid/_createdAt/_permissions when the document explicitly carries $id/$createdAt/$permissions, drop the unconditional _uid = :_newUid from the UPDATE.

Verified: php -l clean on both files. Tests can't be exercised locally (project requires PHP 8.4; runner has 8.3).

Out of scope for this healing pass:

• Operator round-trip on partial updates (the Operator given TypeErrors in testOperator* — refetch path needs investigation; not addressed here).
• Missing required attribute "$createdAt" on testUpsertWithJSONFilters — looks like the createOrUpdate upsert path expects $createdAt that the partial-update branch now strips.
• testManyToOneOneWayRelationship date validation — possibly format-roundtrip issue under PartialStructure.
• Memory/Redis failures look largely pre-existing or environmental (317 Redis errors with Redis server redis:6379 went away).

Commit: (fix): CI — partial update preserves relationships and Postgres adapter

[greptile-apps]

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.

[github-actions]

Claude could not apply patches from: healing (merge conflicts with current branch tip). These tasks need manual attention.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Database/Adapter/MariaDB.php (1)

980-991: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Root cause: Conditional internal metadata mapping can leave SET clause empty.

Both adapters now conditionally add _updatedAt, _uid, _createdAt, and _permissions only when the corresponding offsets exist. When the Database layer performs a permissions-only update ($shouldUpdate=false), none of these offsets may be present, and if no user attributes exist, the SET clause becomes empty.

Consider coordinating the fix across both adapters or addressing this at the Database layer by ensuring at least one column is always present when calling the adapter.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Database/Adapter/MariaDB.php` around lines 980 - 991, The adapter builds
$attributes conditionally from $document->offsetExists checks which can produce
an empty SET clause for permissions-only updates; update
src/Database/Adapter/MariaDB.php so the update builder always supplies at least
one column when composing $attributes — for example, ensure '_updatedAt' is
always added (use $document->getUpdatedAt() if available or a fallback like
current timestamp) or add a dedicated internal no-op field key (e.g.,
'_internal_update_flag') when none of
'_updatedAt','_uid','_createdAt','_permissions' were added; keep the checks for
existing offsets but guarantee $attributes is non-empty before building the SET,
and mirror the same change in the other adapter to keep behavior consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/Database/Adapter/MariaDB.php`:
- Around line 980-991: The adapter builds $attributes conditionally from
$document->offsetExists checks which can produce an empty SET clause for
permissions-only updates; update src/Database/Adapter/MariaDB.php so the update
builder always supplies at least one column when composing $attributes — for
example, ensure '_updatedAt' is always added (use $document->getUpdatedAt() if
available or a fallback like current timestamp) or add a dedicated internal
no-op field key (e.g., '_internal_update_flag') when none of
'_updatedAt','_uid','_createdAt','_permissions' were added; keep the checks for
existing offsets but guarantee $attributes is non-empty before building the SET,
and mirror the same change in the other adapter to keep behavior consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0cd5a620-5c8e-445d-bfe2-b0692881fd19

📥 Commits

Reviewing files that changed from the base of the PR and between 6b2a7b5 and f94cc01.

📒 Files selected for processing (4)

• src/Database/Adapter/MariaDB.php
• src/Database/Adapter/Postgres.php
• src/Database/Adapter/SQLite.php
• src/Database/Database.php

🚧 Files skipped from review as they are similar to previous changes (1)

• src/Database/Database.php

[github-actions]

Claude could not apply patches from: healing (merge conflicts with current branch tip). These tasks need manual attention.

[greptile-apps]

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.