Bulk upsert silently reverts concurrent writes (missing attributes are backfilled from a stale, non-transactional read) #12960

👟 Reproduction steps

1. Have a collection with several optional attributes, e.g. status (string) and score (integer), and a document D.
2. From two server-side clients (API key), run concurrently:
   • Client A: bulk upsert [{ "$id": "D", "score": 42 }] (only score)
   • Client B: bulk upsert [{ "$id": "D", "status": "FT" }] (only status)
3. Repeat under concurrency (or with a batch large enough that the two requests overlap).

Both requests return HTTP 200/201 and report the row as upserted.

👍 Expected behavior

Since each payload contains disjoint attribute sets, both writes should survive: D ends up with score = 42 and status = "FT". This is what happens with two concurrent single updateDocument calls, which are safe (see analysis below).

👎 Actual Behavior

Whichever bulk upsert commits last reverts the other one's columns to the values they had before both requests started — even though those columns were not in its payload, and even though the API reported success for both writes. There is no error, no warning; the lost write is silent.

The root cause is that bulk upsert is not a column-targeted patch. In Database::upsertDocumentsWithIncrease() (utopia-php/database):

1. The old version of each document is read outside the write transaction:

   • in database 3.x (Appwrite 1.8.x): a per-document getDocument(), which is served by the Redis document cache (unbounded TTL), so the merge base can be arbitrarily stale for frequently-read documents;
   • in database 6.x (Appwrite 1.9.x) and current main: a batched find() executed once at the start of the call — a direct DB read, but still outside the write transaction, so it goes stale while earlier chunks of the same call (or concurrent writers) commit.

2. Every optional attribute missing from the payload is then backfilled from that stale read:

   // Force matching optional parameter sets
   // Doesn't use decode as that intentionally skips null defaults to reduce payload size
   foreach ($collectionAttributes as $attr) {
       if (!$attr->getAttribute('required') && !\array_key_exists($attr['$id'], (array)$document)) {
           $document->setAttribute(
               $attr['$id'],
               $old->getAttribute($attr['$id'], ($attr['default'] ?? null))
           );
       }
   }

3. The SQL adapter then writes the full row — getUpsertStatement() builds one INSERT ... ON DUPLICATE KEY UPDATE \col` = VALUES(`col`)` over the union of all columns, including the backfilled ones.

So a bulk upsert that "only touches status" actually rewrites every column, using values read before the concurrent writer committed → classic lost update.

Note the contrast with single updateDocument(), which does this correctly: it reads the old document with getDocument(..., forUpdate: true) inside withTransaction(), so the merge base is row-locked and cannot go stale.

🎲 Real-world impact (how we found it)

Self-hosted Appwrite 1.8.1 (MariaDB), a sports-prediction app. A scheduled job bulk-upserts betting points (homePoints/drawPoints/awayPoints, 35 rows) while another job in the same function bulk-upserts fixture results (~510 rows in chunks of 100) on the same table. MariaDB general log from production, same second:

10:27:43.427  INSERT INTO ..._collection_2 (...) VALUES (..., '1490331', ..., drawPoints = 45, ...)   -- points batch: writes the new value
10:27:43.876  INSERT INTO ..._collection_2 (...) VALUES (..., '1490331', ..., drawPoints = 46, ...)   -- fixtures batch: rewrites the OLD value it read before the first commit

The fixtures batch did not contain any *Points attribute in its payload, yet its SQL rewrote drawPoints with the pre-run value backfilled from its stale read. Both API calls returned success.

The cache-based variant (1.8.x) made it vicious: only hot documents (present in the Redis document cache because users read them constantly) were affected, so the same few rows were silently reverted on every run while their neighbors were fine — it looked like data corruption targeting our most popular records (they were literally the Inter Miami fixtures, i.e. the most-viewed ones).

🤔 Suggested fix

Any of these would remove the silent lost update:

• Read the merge base with row locks inside the same transaction as the write (SELECT ... FOR UPDATE), exactly like single updateDocument() already does; or
• Stop backfilling missing optional attributes from $old, and make the ON DUPLICATE KEY UPDATE clause only assign the columns actually present in each row's payload (true partial upsert — this is what the docs' wording "May contain partial data" suggests is already the case); or
• At minimum, re-read/merge inside the write transaction rather than from a pre-transaction (or cached) snapshot.

🎲 Appwrite version

Version 1.8.x

💻 Operating system

Linux

🧱 Your Environment

• Appwrite 1.8.1 self-hosted (Docker, MariaDB), utopia-php/database 3.5.0 — where the incident happened, with the Redis-cache-served merge base.
• Code path verified unchanged in Appwrite 1.9.5 (utopia-php/database 6.0.0) and in utopia-php/database main at the time of writing (the "Force matching optional parameter sets" backfill loop and the non-transactional old-document read are still there; only the read moved from cached getDocument() to a batched find()).
• Reproduced with the REST API directly (PUT /v1/tablesdb/{db}/tables/{table}/rows) and with dart_appwrite 19.x.

👀 Have you spent some time to check if this issue has been raised before?

• I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

• I have read the Code of Conduct