Skip to content

fix(find): return only the selected attributes of a distinct row - #1032

Merged
abnegate merged 1 commit into
mainfrom
fix/distinct-row-id
Oct 16, 2026
Merged

abnegate merged 1 commit into
mainfrom
fix/distinct-row-id

Conversation

@abnegate

Copy link
Copy Markdown
Member

What

On staging, a distinct read with ttl worked on the cache miss but returned 400 "$id must be of type string" on every cache hit.

The root cause is in the library. find() decodes each row with the read's selections, and validateSelections() always appended $id, $sequence, $collection, $createdAt, $updatedAt and $permissions to them. A distinct read selects only what its select() names, so decode() set each internal attribute it never read to null. A row from find([Query::distinct(), Query::select(['k'])]) therefore carried $id => null, and new Document($row->getArrayCopy()) refused it. Appwrite's list cache rebuilds cached rows exactly that way.

validateSelections() no longer appends the internal attributes for a distinct read. A distinct row carries only what it selects, plus $collection, and carries $id only when the read selects it. Non-distinct reads are unchanged and still carry $id and the other internal attributes. Every row find() returns now round-trips through new Document($row->getArrayCopy()).

Regression tests

Test Adapters origin/main this PR
DocumentTests::testDistinctRowsCarryOnlyWhatTheySelectAndRebuildIntoADocument: distinct of k (including a null value), of k+n, of k+$createdAt, of k+$id, distinct pages by cursor, and a non-distinct select; each row round-trips through new Document() MySQL, MariaDB, Postgres, SQLite error: Utopia\Database\Exception\Structure: $id must be of type string pass
MongoDB, Memory, Redis pass (distinct is refused with Exception\Query; a non-distinct select round-trips) pass

Run locally against Docker: the full adapter suites for MySQL, MariaDB, Postgres, SQLite, MongoDB, Memory and Redis pass, as do the full unit suite, Pint and PHPStan.

API repro (Appwrite)

GET /v1/tablesdb/{db}/tables/{t}/rows?queries[]={"method":"distinct"}&queries[]={"method":"select","values":["k"]}&ttl=60, called twice → both calls 200; the second has X-Appwrite-Cache: hit. Rows have no $id key unless select names $id (was 400 "$id must be of type string" on the hit). With select(["k","$id"]) every row carries a string $id.

🤖 Generated with Claude Code

find() decoded every row with the read's selections, to which
validateSelections() always appended $id, $sequence, $collection,
$createdAt, $updatedAt and $permissions so a document read with a select
still carries them. A distinct read selects only what its select names,
so decode() set each of those it never read to null, and a distinct row
carried `$id => null`. The library handed out a Document its own
constructor refuses: `new Document($row->getArrayCopy())` threw "$id must
be of type string", which is how Appwrite's list cache rebuilt a cached
distinct read and answered 400 on every cache hit.

A distinct read now decodes only the attributes it selects. A distinct row
carries $id only when the read selects it, and every row find() returns
rebuilds into an equal Document.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: utopia-php/database/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 58eb07db-9c9a-457d-97cb-5c8c8cd681be
📥 Commits

Reviewing files that changed from the base of the PR and between b1ab94f and bb10697.

📒 Files selected for processing: 2
  • src/Database/Trait/Documents.php
  • tests/e2e/Adapter/Scopes/DocumentTests.php
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@abnegate
abnegate merged commit b969d69 into main Oct 16, 2026
24 checks passed
@abnegate
abnegate deleted the fix/distinct-row-id branch October 16, 2026 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant