Object attribute query and index #785
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📝 WalkthroughWalkthroughAdds support for nested object attribute paths (dot notation) across adapters and validators: index creation, query conversion, and filtering for dotted object attributes in MongoDB and PostgreSQL, plus validator updates and extensive tests for nested-path indexing and querying. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Database
participant Validator
participant Adapter
participant Engine
Client->>Database: createIndex("profile.user.email", type)
Database->>Validator: checkValidIndex("profile.user.email", supportForObjects)
Validator->>Validator: extract base "profile" and verify type == OBJECT
Validator-->>Database: valid / invalid
Database->>Adapter: createIndex(processed expression)
Adapter->>Adapter: build expression (Mongo dot-path / Postgres JSONB via buildJsonbPath)
Adapter->>Engine: CREATE INDEX on expression
Engine-->>Adapter: result
Adapter-->>Database: success
Database-->>Client: index created
sequenceDiagram
participant Client
participant Database
participant QueryConverter
participant Validator
participant Adapter
participant Engine
Client->>Database: query(profile.user.email = "a@b.com")
Database->>QueryConverter: detect dotted attribute
QueryConverter->>QueryConverter: derive base "profile", mark as OBJECT
QueryConverter-->>Database: set attribute type = OBJECT (dotted)
Database->>Validator: validate filter as TEXT for dotted object path
Validator-->>Database: valid
Database->>Adapter: build filter (JSONB/text or Mongo dot access)
Adapter->>Engine: execute query with nested path expression
Engine-->>Adapter: results
Adapter-->>Database: documents
Database-->>Client: filtered results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Database/Validator/Query/Filter.php (1)
86-189: Bug: dotted-object path values skip validation due tocontinue 2.When
$isDottedOnObjectis true you create aTextvalidator butcontinue 2;bypasses the actual$validator->isValid($value)check. This can let non-strings through and blow up later in adapters (e.g., string ops for LIKE/regex).Proposed fix
@@ case Database::VAR_OBJECT: // For dotted attributes on objects, validate as string (path queries) if ($isDottedOnObject) { $validator = new Text(0, 0); - continue 2; + break; } - // object containment queries on the base object attribute - elseif (\in_array($method, [Query::TYPE_EQUAL, Query::TYPE_NOT_EQUAL, Query::TYPE_CONTAINS, Query::TYPE_NOT_CONTAINS], true) + // object containment queries on the base object attribute + if (\in_array($method, [Query::TYPE_EQUAL, Query::TYPE_NOT_EQUAL, Query::TYPE_CONTAINS, Query::TYPE_NOT_CONTAINS], true) && !$this->isValidObjectQueryValues($value)) { $this->message = 'Invalid object query structure for attribute "' . $attribute . '"'; return false; } continue 2;
🤖 Fix all issues with AI agents
In `@src/Database/Validator/Index.php`:
- Around line 768-776: Both helper methods lack return type declarations causing
PHPStan failures; add explicit return types: change isDottedAttribute(string
$attribute) to return bool and change getBaseAttributeFromDottedAttribute(string
$attribute) to return string (i.e., add ": bool" and ": string" to the
respective method signatures), keeping the current logic intact.
- Around line 169-180: The current loop in Index.php incorrectly iterates over
all index attributes and enforces object-type on non-dotted attributes; change
the loop to iterate only over $dottedAttributes (the result of array_filter) so
the object-type check runs only for dotted attributes, derive $baseAttribute via
getBaseAttributeFromDottedAttribute($attribute) as already intended, and keep
the existing error message/return false when the base attribute exists but is
not Database::VAR_OBJECT.
In `@tests/e2e/Adapter/Scopes/ObjectAttributeTests.php`:
- Around line 1180-1190: Remove the redundant if-check around the object-index
test: the outer test already skipped when getSupportForObjectIndexes() is false,
so delete the conditional "if
($database->getAdapter()->getSupportForObjectIndexes()) {" and its matching
closing brace, leaving the try/catch that creates the index
'idx_profile_nested_object' with Database::INDEX_OBJECT and the subsequent
assertions intact; make sure the block still catches Exception $e and asserts
InstanceOf IndexException and that $exceptionThrown is asserted true.
🧹 Nitpick comments (4)
src/Database/Adapter/Postgres.php (1)
1751-1772: Good nested-path handling; consider aliasingSEARCH/NOT_SEARCHfor consistency.
filterObjectPath()+ skippinghandleObjectQueries()for nested paths makes sense. One gotcha:TYPE_SEARCH/TYPE_NOT_SEARCHusesregexp_replace({$attribute}, ...)without the table alias, which can become ambiguous in joined queries; using{$alias}.{$attribute}would be safer for both simple and nested paths.Proposed tweak
@@ case Query::TYPE_SEARCH: $binds[":{$placeholder}_0"] = $this->getFulltextValue($query->getValue()); - return "to_tsvector(regexp_replace({$attribute}, '[^\w]+',' ','g')) @@ websearch_to_tsquery(:{$placeholder}_0)"; + return "to_tsvector(regexp_replace({$alias}.{$attribute}, '[^\w]+',' ','g')) @@ websearch_to_tsquery(:{$placeholder}_0)"; case Query::TYPE_NOT_SEARCH: $binds[":{$placeholder}_0"] = $this->getFulltextValue($query->getValue()); - return "NOT (to_tsvector(regexp_replace({$attribute}, '[^\w]+',' ','g')) @@ websearch_to_tsquery(:{$placeholder}_0))"; + return "NOT (to_tsvector(regexp_replace({$alias}.{$attribute}, '[^\w]+',' ','g')) @@ websearch_to_tsquery(:{$placeholder}_0))";tests/unit/Validator/IndexTest.php (1)
328-439: Test coverage is solid; small readability nits.
$validNestedObjectIndexis assertedfalse→ rename to$invalidNestedObjectIndex.- Consider using named args for the
new Index(...)constructor here (positional booleans are brittle).Minimal rename
@@ - $validNestedObjectIndex = new Document([ + $invalidNestedObjectIndex = new Document([ '$id' => ID::custom('idx_nested_object'), 'type' => Database::INDEX_OBJECT, 'attributes' => ['data.key.nestedKey'], 'lengths' => [], 'orders' => [], ]); - $this->assertFalse($validator->isValid($validNestedObjectIndex)); + $this->assertFalse($validator->isValid($invalidNestedObjectIndex));src/Database/Database.php (2)
8147-8162: Minor perf/clarity: compute the base attribute once for nested-object query inference.Right now
explode()happens inside the loop; you can compute$baseAttributeonce and reuse$queryAttributeconsistently.
3638-3715: Make base-attribute matching more robust for dot-path indexes (key fallback + case-insensitive compare).
validateAttribute()treats attribute IDs case-insensitively, butcreateIndex()matches$baseAttragainstkeywith a case-sensitive comparison. That can leave$indexAttributesWithTypesincomplete (and skip length/array adjustments) if callers pass a different case, or if an attribute lackskey.Proposed tweak (more defensive matching)
- $baseAttr = $attr; - if (\str_contains($attr, '.')) { - $baseAttr = \explode('.', $attr, 2)[0] ?? $attr; - } + $baseAttr = \str_contains($attr, '.') + ? \explode('.', $attr, 2)[0] + : $attr; + $baseAttrLc = \strtolower($baseAttr); foreach ($collectionAttributes as $collectionAttribute) { - if ($collectionAttribute->getAttribute('key') === $baseAttr) { + $collectionAttrKey = (string)$collectionAttribute->getAttribute('key', $collectionAttribute->getId()); + if (\strtolower($collectionAttrKey) === $baseAttrLc) { $attributeType = $collectionAttribute->getAttribute('type'); $indexAttributesWithTypes[$attr] = $attributeType;#!/bin/bash set -euo pipefail # Ensure Adapter::createIndex signature/arity matches this call (and all implementations are updated). rg -nP '\bfunction\s+createIndex\s*\(' rg -nP '->createIndex\s*\('
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Database/Adapter/Mongo.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Database.phpsrc/Database/Validator/Index.phpsrc/Database/Validator/Query/Filter.phptests/e2e/Adapter/Scopes/ObjectAttributeTests.phptests/e2e/Adapter/Scopes/SchemalessTests.phptests/unit/Validator/IndexTest.php
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-10-03T02:04:17.803Z
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.
Applied to files:
tests/unit/Validator/IndexTest.phpsrc/Database/Adapter/Mongo.phptests/e2e/Adapter/Scopes/SchemalessTests.phptests/e2e/Adapter/Scopes/ObjectAttributeTests.php
📚 Learning: 2025-10-29T12:27:57.071Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 747
File: src/Database/Adapter/Mongo.php:1449-1453
Timestamp: 2025-10-29T12:27:57.071Z
Learning: In src/Database/Adapter/Mongo.php, when getSupportForAttributes() returns false (schemaless mode), the updateDocument method intentionally uses a raw document without $set operator for replacement-style updates, as confirmed by the repository maintainer ArnabChatterjee20k.
Applied to files:
src/Database/Adapter/Mongo.phptests/e2e/Adapter/Scopes/SchemalessTests.php
📚 Learning: 2025-10-03T01:50:11.943Z
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/AttributeTests.php:1329-1334
Timestamp: 2025-10-03T01:50:11.943Z
Learning: MongoDB has a 1024kb (1,048,576 bytes) limit for index entries. The MongoDB adapter's getMaxIndexLength() method should return this limit rather than 0.
Applied to files:
src/Database/Adapter/Mongo.php
📚 Learning: 2025-11-25T10:46:37.666Z
Learnt from: fogelito
Repo: utopia-php/database PR: 763
File: src/Database/Database.php:8671-8751
Timestamp: 2025-11-25T10:46:37.666Z
Learning: In Utopia\Database\Database::processRelationshipQueries, ensure the system select for '$id' is added only when the collection has relationship attributes (i.e., !empty($relationships)), and avoid capturing the unused '$idAdded' from Query::addSelect to satisfy static analysis.
Applied to files:
src/Database/Database.php
📚 Learning: 2025-10-20T05:29:29.487Z
Learnt from: abnegate
Repo: utopia-php/database PR: 731
File: src/Database/Database.php:6987-6988
Timestamp: 2025-10-20T05:29:29.487Z
Learning: In Utopia\Database\Database::convertRelationshipQueries, Many-to-Many filtering does not need the parent collection or a direct junction query: it calls find() on the related collection without skipping relationships, which populates relationship attributes (including the two-way key). Parent IDs are derived from that populated attribute. Therefore, calls should remain convertRelationshipQueries($relationships, $queries).
Applied to files:
src/Database/Database.php
📚 Learning: 2025-10-16T09:37:33.531Z
Learnt from: fogelito
Repo: utopia-php/database PR: 733
File: src/Database/Adapter/MariaDB.php:1801-1806
Timestamp: 2025-10-16T09:37:33.531Z
Learning: In the MariaDB adapter (src/Database/Adapter/MariaDB.php), only duplicate `_uid` violations should throw `DuplicateException`. All other unique constraint violations, including `PRIMARY` key collisions on the internal `_id` field, should throw `UniqueException`. This is the intended design to distinguish between user-facing document duplicates and internal/user-defined unique constraint violations.
Applied to files:
tests/e2e/Adapter/Scopes/ObjectAttributeTests.php
📚 Learning: 2025-07-01T11:31:37.438Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 613
File: src/Database/Adapter/Postgres.php:1254-1319
Timestamp: 2025-07-01T11:31:37.438Z
Learning: In PostgreSQL adapter methods like getUpsertStatement, complexity for database-specific SQL generation is acceptable when the main business logic is properly separated in the parent SQL adapter class, following the adapter pattern where each database adapter handles its own SQL syntax requirements.
Applied to files:
src/Database/Adapter/Postgres.php
📚 Learning: 2025-07-30T19:17:53.630Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 642
File: src/Database/Validator/PartialStructure.php:43-52
Timestamp: 2025-07-30T19:17:53.630Z
Learning: In PartialStructure validator, when filtering for required attributes validation using the $requiredAttributes parameter, $this->attributes should be used instead of the merged $attributes array because this validation is specifically for internal attributes like $createdAt and $updatedAt that are defined in the base Structure class, not collection-specific attributes.
Applied to files:
src/Database/Validator/Query/Filter.php
🧬 Code graph analysis (6)
src/Database/Validator/Index.php (1)
src/Database/Database.php (1)
Database(37-8812)
src/Database/Adapter/Mongo.php (2)
src/Database/Adapter.php (1)
filter(1255-1264)src/Database/Adapter/SQL.php (1)
getInternalKeyForAttribute(2383-2395)
tests/e2e/Adapter/Scopes/SchemalessTests.php (1)
src/Database/Adapter/Mongo.php (3)
getSupportForAttributes(2792-2795)createCollection(408-572)createIndex(907-1034)
tests/e2e/Adapter/Scopes/ObjectAttributeTests.php (6)
src/Database/Database.php (2)
Database(37-8812)getDatabase(914-917)src/Database/Adapter.php (2)
getDatabase(162-165)getSupportForObjectIndexes(1087-1087)src/Database/Adapter/Postgres.php (1)
getSupportForObjectIndexes(2241-2244)src/Database/Adapter/MySQL.php (1)
getSupportForObjectIndexes(256-259)src/Database/Adapter/SQLite.php (1)
getSupportForObjectIndexes(1021-1024)src/Database/Adapter/MariaDB.php (1)
getSupportForObjectIndexes(2148-2151)
src/Database/Adapter/Postgres.php (3)
src/Database/Adapter.php (2)
filter(1255-1264)quote(469-469)src/Database/Query.php (2)
isObjectAttribute(999-1002)getAttribute(182-185)src/Database/Document.php (1)
getAttribute(224-231)
src/Database/Validator/Query/Filter.php (2)
src/Database/Database.php (1)
Database(37-8812)src/Database/Query.php (1)
Query(8-1237)
🪛 GitHub Actions: CodeQL
src/Database/Validator/Index.php
[error] 768-768: isDottedAttribute() has no return type specified. (PHPStan: missing return type)
[error] 773-773: getBaseAttributeFromDottedAttribute() has no return type specified. (PHPStan: missing return type)
tests/e2e/Adapter/Scopes/ObjectAttributeTests.php
[error] 1181-1181: If condition is always true. (PHPStan: always-true conditional)
⏰ 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). (13)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (Schemaless/MongoDB)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (SharedTables/MongoDB)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (MongoDB)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (Postgres)
🔇 Additional comments (7)
src/Database/Adapter/Postgres.php (1)
884-899: No action needed. The code correctly handles nested path indexing. The$indexAttributesWithTypesparameter passed fromDatabase.php::createIndex()is keyed by the full dotted attribute path, so theisset($indexAttributeTypes[$attr])check inPostgres.phpworks as intended.src/Database/Validator/Index.php (3)
251-266: LGTM!The validation logic correctly allows dotted attributes when object indexes are supported and the base attribute exists.
401-403: LGTM!Correctly resolves dotted attributes to their base for index length validation.
749-755: LGTM!Correctly enforces that
INDEX_OBJECTcan only be created on top-level object attributes, preventing invalid GIN indexes on nested paths.tests/e2e/Adapter/Scopes/SchemalessTests.php (1)
1869-1958: LGTM!Comprehensive test coverage for dot-notation indexing in schemaless adapters:
- Correctly gates test for schemaless-only execution
- Validates both KEY and UNIQUE index creation on nested paths
- Verifies unique constraint enforcement with expected
DuplicateException- Tests dot-notation query functionality
tests/e2e/Adapter/Scopes/ObjectAttributeTests.php (2)
8-8: LGTM!Import correctly added for
DuplicateExceptionused in the new tests.
1205-1333: LGTM!Excellent test coverage for querying nested object attributes:
- Tests multiple query operators (
equal,startsWith,endsWith,contains,notEndsWith)- Validates logical combinations (
AND,OR)- Properly seeds test data with varied nested structures
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…in Postgres adapter
… adapter and update attribute handling in Postgres validator
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Database/Database.php`:
- Around line 2872-2895: The IndexValidator instantiation block is over-indented
under the surrounding if ($altering) scope causing a Pint PSR-12
statement_indentation failure; reformat the block so its indentation aligns with
the surrounding code style (match other method calls/constructors in the file),
ensuring the if ($this->validate) { ... } block and the new IndexValidator(...)
call use the correct 4-space indentation and argument alignment for PSR-12
(e.g., arguments indented one level inside the parentheses and the closing );
aligned with the opening line) so Pint passes; update the lines around the
IndexValidator, $this->validate check, and the surrounding if ($altering) scope
accordingly.
In `@tests/e2e/Adapter/Scopes/ObjectAttributeTests.php`:
- Around line 1545-1558: The local variable $updated returned from the call to
$database->updateDocument($collectionId, 'mixed1', new Document(...)) is unused
and flagged by PHPMD; either remove the assignment and call
$database->updateDocument(...) directly, or keep the assignment and make use of
$updated (for example assert its contents or check fields) so it is referenced;
locate the updateDocument call and the $updated variable in the test (variable
name $updated) and apply one of these two fixes to eliminate the unused variable
warning.
abnegate
reviewed
Feb 11, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Tests