Primary bigint

[fogelito]

Summary by CodeRabbit

• New Features

  • ID columns expanded to 64-bit and migrated to modern identity-style auto-increment, enabling support for very large document IDs and more consistent auto-increment behavior across databases.

• Tests

  • Added end-to-end test verifying creation, retrieval, and correct auto-increment progression when using extremely large ID/sequence values.

[coderabbitai]

Walkthrough

Primary key column types were widened to 64-bit: MariaDB _id columns changed to BIGINT UNSIGNED AUTO_INCREMENT; Postgres _id columns changed to BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY and the _permissions column position adjusted. An end-to-end test for very large sequence values was added.

Changes

Cohort / File(s)	Summary
MariaDB DDL update src/Database/Adapter/MariaDB.php	Changed _id column in collection and permissions tables from INT(11) UNSIGNED NOT NULL AUTO_INCREMENT to BIGINT UNSIGNED NOT NULL AUTO_INCREMENT.
Postgres DDL update src/Database/Adapter/Postgres.php	Replaced SERIAL NOT NULL _id with BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY; removed separate PRIMARY KEY (_id) constraint; moved _permissions column to appear after dynamic attribute definitions.
E2E test for bigint sequences tests/e2e/Adapter/Scopes/DocumentTests.php	Added testBigintSequence to insert a very large sequence value (5,000,000,000,000,000) and verify retrieval and subsequent auto-increment behavior.

Sequence Diagram(s)

Changes are limited to DDL adjustments and a test; there is no runtime control-flow or interaction change to illustrate, so no sequence diagram is provided.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• abnegate

Poem

Thump-thump, I hop through code and cheer,
IDs grew tall, no need to fear.
Maria and Postgres stretch so wide,
Tests count high with rabbit pride.
Carrots for CI—let's glide! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Primary bigint" is concise and directly matches the main change in the changeset—migrating collection primary key columns to BIGINT in the MariaDB and Postgres adapters and adding a test for very large sequences—so it communicates the primary intent without extra noise. It is specific enough for a reviewer scanning PR history to understand the core change. The title is brief and avoids irrelevant details.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch primary-bigint

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89a5ec6 and 6c5b7df.

📒 Files selected for processing (1)

• tests/e2e/Adapter/Scopes/DocumentTests.php (1 hunks)

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

• tests/e2e/Adapter/Scopes/DocumentTests.php

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

• GitHub Check: Adapter Tests (SharedTables/Postgres)
• GitHub Check: Adapter Tests (SharedTables/MariaDB)
• GitHub Check: Adapter Tests (Postgres)
• GitHub Check: Adapter Tests (MySQL)
• GitHub Check: Adapter Tests (SharedTables/SQLite)
• GitHub Check: Adapter Tests (Pool)
• GitHub Check: Adapter Tests (MariaDB)
• GitHub Check: Adapter Tests (Mirror)
• GitHub Check: Adapter Tests (SharedTables/MySQL)
• GitHub Check: Adapter Tests (SQLite)
• GitHub Check: Unit Test

---

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.}

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

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

263-268: Same version dependency applies to the permissions table

The permissions table now uses the same BIGINT … IDENTITY construct and therefore shares the version requirement raised above.

🧹 Nitpick comments (1)

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

188-188: Consider making permissions table PK unsigned for consistency
The main collection uses BIGINT UNSIGNED for its _id, but the permissions table uses signed BIGINT. Converting it to BIGINT UNSIGNED would maintain consistency across tables.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2bd87ac and bf59887.

⛔ Files ignored due to path filters (1)

• composer.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

• docker-compose.yml (2 hunks)
• src/Database/Adapter/MariaDB.php (2 hunks)
• src/Database/Adapter/Postgres.php (3 hunks)
• tests/e2e/Adapter/Base.php (1 hunks)
• tests/e2e/Adapter/Scopes/DocumentTests.php (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

src/Database/Adapter/Postgres.php (2)

src/Database/Document.php (1)

• getSequence (67-70)

src/Database/Adapter/SQL.php (2)

• getPDO (1532-1535)
• getSQLTable (1523-1526)

⏰ Context from checks skipped due to timeout of 90000ms (1)

• GitHub Check: Setup & Build Docker Image

🔇 Additional comments (6)

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

157-157: Approve primary key upgrade to BIGINT UNSIGNED
The change from INT(11) UNSIGNED to BIGINT UNSIGNED for the _id column ensures support for larger auto-increment values while preserving unsigned semantics.

tests/e2e/Adapter/Base.php (1)

21-23: Approve trait reordering for bigint sequence tests
Moving CollectionTests after DocumentTests aligns with the new testBigintSequence addition and improves logical grouping of E2E scopes.

docker-compose.yml (1)

31-31: Upgrade PostgreSQL service images to 17.5
Updating both postgres and postgres-mirror to postgres:17.5 aligns the test environment with the new PostgreSQL adapter changes.

Also applies to: 42-42

tests/e2e/Adapter/Scopes/DocumentTests.php (1)

31-52: ```shell
#!/bin/bash

Check if ext-bcmath is declared as a requirement

grep -R '"ext-bcmath"' -n composer.json

Check for any usage of bcadd or other bcmath functions in the codebase

grep -R "bcadd" -n .
grep -R "bcmul|bcsub|bcdiv" -n .

</details>
<details>
<summary>src/Database/Adapter/Postgres.php (2)</summary>

`232-241`: **BIGINT IDENTITY assumes Postgres ≥ 10 – please confirm runtime compatibility**

The new `_id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY` syntax is only accepted by PostgreSQL 10+.  
If any environments (CI, on-prem customers, etc.) are still on 9.​6 or earlier this DDL will fail at collection creation time.


Would you double-check the lowest Postgres version we officially support and, if needed, guard this with a compatibility branch or update the docs?

---

`1030-1039`: **Re-syncing the sequence: verify `pg_get_serial_sequence()` works with IDENTITY columns**

After switching to IDENTITY, `pg_get_serial_sequence()` is called to reset the underlying sequence when a manual `$sequence` is supplied.  
Although Postgres does create a hidden sequence for IDENTITY columns, older drivers and versions (< 12) have occasionally returned `NULL` for this function.


```shell
#!/bin/bash
# Sanity-check that pg_get_serial_sequence returns a sequence name for an IDENTITY column
# and that setval succeeds.  Expect: a non-NULL sequence name printed.
psql -Atc "CREATE TEMP TABLE tmp(_id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY);" &&
psql -Atc "SELECT pg_get_serial_sequence('tmp', '_id');"

If the command prints an empty line, we’ll need an alternative (e.g. pg_get_identity_sequence() in v16+ or explicit ALTER SEQUENCE).

[abnegate]

Let's keep it unsigned

[fogelito]

Done!