Skip to content

Conversation

@abnegate
Copy link
Member

@abnegate abnegate commented Sep 26, 2025

Summary by CodeRabbit

  • New Features

    • Added support for random sorting in queries, enabling results to be returned in a random order.
    • Introduced capability detection for adding optional spatial attributes to collections with existing data; support varies by database adapter.
  • Bug Fixes

    • Improved MySQL error reporting by converting a specific database error into a clearer validation message when an attribute disallows null values.
    • Refined transaction error handling to ensure consistent behavior after rollbacks.
  • Tests

    • Added end-to-end and unit tests for random ordering and spatial attribute behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Introduces random ordering support across query, validation, SQL adapters, and tests; adds adapter capability probe getSupportForOptionalSpatialAttributeWithExistingRows() and implements per-adapter responses; adjusts MySQL exception mapping; tweaks Adapter transaction rollback error dispatch flow; and adds spatial behavior tests.

Changes

Cohort / File(s) Summary
Adapter core & transactions
src/Database/Adapter.php
Moves post-rollback exception handling outside the rollback try-catch in transaction flow; adds abstract method getSupportForOptionalSpatialAttributeWithExistingRows(): bool.
Ordering: RANDOM support (API, parsing, validation, base SQL, tests)
src/Database/Database.php, src/Database/Query.php, src/Database/Validator/Queries.php, src/Database/Validator/Query/Order.php, src/Database/Adapter/SQL.php, tests/unit/QueryTest.php, tests/e2e/Adapter/Scopes/DocumentTests.php
Adds ORDER_RANDOM constant and Query::orderRandom(); parser/validator recognize TYPE_ORDER_RANDOM; SQL base adds abstract getRandomOrder() and handles ORDER_RANDOM in find(); unit/e2e tests cover random ordering (note: test duplicated in DocumentTests).
Adapter implementations: RANDOM function + spatial capability probe
src/Database/Adapter/MariaDB.php, src/Database/Adapter/Postgres.php, src/Database/Adapter/SQLite.php
Adds protected getRandomOrder() returning RAND() (MariaDB) or RANDOM() (Postgres/SQLite); implements getSupportForOptionalSpatialAttributeWithExistingRows() returning true (MariaDB/SQLite) or false (Postgres).
MySQL adapter: error mapping + spatial capability probe
src/Database/Adapter/MySQL.php
Maps PDO HY000/1138 to StructureException (attribute does not allow null); implements getSupportForOptionalSpatialAttributeWithExistingRows() returning false.
Pool delegation
src/Database/Adapter/Pool.php
Adds delegated getSupportForOptionalSpatialAttributeWithExistingRows(): bool.
Spatial tests
tests/e2e/Adapter/Scopes/SpatialTests.php
Adds test for creating spatial column with existing data, asserting StructureException when unsupported.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Query
  participant Database
  participant SQLAdapter as SQL Adapter
  participant RDBMS as DB Engine

  Client->>Query: Query::orderRandom()
  Client->>Database: find(collection, [orderRandom, ...])
  Database->>SQLAdapter: build+execute SELECT
  SQLAdapter->>SQLAdapter: detect ORDER_RANDOM
  SQLAdapter->>SQLAdapter: getRandomOrder() -> RAND()/RANDOM()
  SQLAdapter->>RDBMS: SELECT ... ORDER BY RAND()/RANDOM()
  RDBMS-->>SQLAdapter: rows
  SQLAdapter-->>Database: documents
  Database-->>Client: result set
Loading
sequenceDiagram
  autonumber
  participant App as Caller
  participant Adapter
  participant PDO as PDO Connection

  App->>Adapter: withTransaction(fn, retries)
  Adapter->>PDO: BEGIN
  Adapter->>Adapter: execute user callback
  alt error during callback
    Adapter->>PDO: ROLLBACK (try)
    PDO-->>Adapter: result or error
    note over Adapter: Post-rollback exception dispatch moved\noutside inner try-catch
    Adapter->>App: rethrow/mapped exception
    else success
      Adapter->>PDO: COMMIT
      Adapter-->>App: result
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • fogelito

Poem

A hop, a skip—RANDOM order I choose,
Shuffling rows like carrots I muse.
Spatial fields checked, ears alert,
Rollbacks tidy—no dirt on my shirt.
MySQL grumbles? I map it with care.
Thump-thump! Ship it—bugs beware. 🥕🐇

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 1.x

📜 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 daf0276 and c141d87.

📒 Files selected for processing (14)
  • src/Database/Adapter.php (2 hunks)
  • src/Database/Adapter/MariaDB.php (2 hunks)
  • src/Database/Adapter/MySQL.php (3 hunks)
  • src/Database/Adapter/Pool.php (1 hunks)
  • src/Database/Adapter/Postgres.php (2 hunks)
  • src/Database/Adapter/SQL.php (2 hunks)
  • src/Database/Adapter/SQLite.php (1 hunks)
  • src/Database/Database.php (1 hunks)
  • src/Database/Query.php (5 hunks)
  • src/Database/Validator/Queries.php (1 hunks)
  • src/Database/Validator/Query/Order.php (1 hunks)
  • tests/e2e/Adapter/Scopes/DocumentTests.php (1 hunks)
  • tests/e2e/Adapter/Scopes/SpatialTests.php (1 hunks)
  • tests/unit/QueryTest.php (5 hunks)

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 and usage tips.

@abnegate abnegate marked this pull request as ready for review September 26, 2025 02:23
@abnegate abnegate merged commit 19a3ab2 into main Sep 26, 2025
14 of 15 checks passed
@abnegate abnegate deleted the 1.x branch September 26, 2025 02:23
This was referenced Sep 28, 2025
@coderabbitai coderabbitai bot mentioned this pull request Oct 6, 2025
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.

4 participants