feat(upsert): remove _uid from update checks and add test for upsert …

[ArnabChatterjee20k]

…on unique index

[coderabbitai]

Warning

Rate limit exceeded

@ArnabChatterjee20k has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 32 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ea006084-6dd6-47d7-9112-df876043da4b

📥 Commits

Reviewing files that changed from the base of the PR and between eb35e68 and b403cac.

📒 Files selected for processing (4)

• src/Database/Adapter/MariaDB.php
• src/Database/Adapter/Postgres.php
• src/Database/Adapter/SQLite.php
• tests/e2e/Adapter/Scopes/DocumentTests.php

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch upsert-update-uids

---

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 allows _uid to be overwritten during an upsert when a secondary unique-index conflict (e.g. on email) is the trigger, rather than always treating _uid as an immutable field. The change lands in all three SQL adapters, but only has a functional effect in MariaDB — Postgres and SQLite use ON CONFLICT ("_uid") so the added _uid = excluded._uid assignment is always a value-preserving no-op at conflict time.

• MariaDB: _uid removed from the ON DUPLICATE KEY UPDATE exclusion list; when any unique-index fires the update branch, the row's _uid is now replaced with the proposed value, enabling the "upsert by secondary unique index" pattern the new test exercises.
• Postgres / SQLite: Same one-line change but functionally inert — conflict detection remains bound to _uid, so excluded._uid equals the existing value and the SET assignment changes nothing.
• New test (testUpsertOnUniqueIndexCanUpdateUid): verifies the old _uid (original-id) disappears, the new one (updated-id) takes its place, the auto-increment sequence is preserved (confirming an in-place UPDATE, not a DELETE+INSERT), and only one row remains; correctly skipped on Postgres and SQLite via getSupportForUpsertOnUniqueIndex().

Confidence Score: 5/5

Safe to merge — the functional change is limited to MariaDB's ON DUPLICATE KEY UPDATE path, the Postgres and SQLite adapters are unaffected at runtime, and the new test covers the intended scenario end-to-end.

The adapter edits are minimal and well-scoped: removing _uid from the exclusion list in MariaDB correctly enables the new feature, and the equivalent change in Postgres/SQLite produces a self-assignment that leaves row state unchanged. The new test validates the full lifecycle — document creation, upsert on a secondary unique index, _uid replacement, sequence preservation, and final document count.

No files require special attention.

Important Files Changed

Filename	Overview
src/Database/Adapter/MariaDB.php	Removes _uid from the upsert column exclusion list so ON DUPLICATE KEY UPDATE now propagates the new _uid when a secondary unique-index conflict fires — the intended behavior for this feature.
src/Database/Adapter/Postgres.php	Same exclusion-list change as MariaDB, but Postgres uses ON CONFLICT ("_uid") DO UPDATE, so excluded._uid always equals the existing _uid at conflict time — the added _uid = excluded._uid is a value-preserving no-op with no behavioral change.
src/Database/Adapter/SQLite.php	Same one-line change as Postgres; SQLite also uses ON CONFLICT (_uid) DO UPDATE, making the added _uid assignment a no-op. getSupportForUpsertOnUniqueIndex() returns false here, so the new test is correctly skipped.
tests/e2e/Adapter/Scopes/DocumentTests.php	Adds testUpsertOnUniqueIndexCanUpdateUid which verifies MariaDB can overwrite _uid on an email-unique-index collision, preserves the auto-increment sequence, and leaves exactly one document. Correctly gated by getSupportForUpsertOnUniqueIndex().

_{Reviews (2): Last reviewed commit: "Merge branch 'main' into upsert-update-u..." | Re-trigger Greptile}

[greptile-apps]

Missing adapter method — test will throw a fatal error

getSupportForUpsertOnUniqueIndex() is called here but is not declared in src/Database/Adapter.php (which defines all getSupportFor* abstract methods) and is not implemented in any adapter class. PHP will throw a fatal Call to undefined method error when this test runs, so it can never reach the assertion block or even skip gracefully. The method needs to be added as an abstract declaration in Adapter.php and implemented in each concrete adapter (returning true for MariaDB, which uses ON DUPLICATE KEY UPDATE for any unique constraint, and false for Postgres/SQLite, which use ON CONFLICT ("_uid") and cannot catch conflicts on other unique indexes).

[ArnabChatterjee20k]

no its present

[greptile-apps]

You're right, my apologies — getSupportForUpsertOnUniqueIndex() is present in Adapter.php and all the concrete adapters. My comment was incorrect.

Tip: You can give me more context with .greptile/rules.md so I get this right next time.