Conversation
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
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 |
|
|
|
||
| // Process in chunks to avoid exceeding query value limits | ||
| foreach (\array_chunk($uniqueRelatedIds, \max(1, $this->maxQueryValues)) as $chunk) { | ||
| foreach (\array_chunk($uniqueRelatedIds, self::RELATION_QUERY_CHUNK_SIZE) as $chunk) { |
There was a problem hiding this comment.
Configured query limit ignored
When setMaxQueryValues(5) is used, populating six or more unique relationship IDs now creates a chunk larger than the configured limit. The internal find() call validates that chunk against the same limit and throws QueryException, so an otherwise valid relationship read fails instead of being split into executable queries. The same fixed chunk size is used in the other relationship-loading paths at lines 5420, 5517, 5596, and 5626.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Database/Database.php
Line: 5328
Comment:
**Configured query limit ignored**
When `setMaxQueryValues(5)` is used, populating six or more unique relationship IDs now creates a chunk larger than the configured limit. The internal `find()` call validates that chunk against the same limit and throws `QueryException`, so an otherwise valid relationship read fails instead of being split into executable queries. The same fixed chunk size is used in the other relationship-loading paths at lines 5420, 5517, 5596, and 5626.
**Knowledge Base Used:**
- [Query construction and execution](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/utopia-php/database/-/docs/query-execution.md)
- [Storage adapter architecture](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/utopia-php/database/-/docs/adapter-architecture.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| @@ -26,91 +26,6 @@ trait RelationshipTests | |||
| use ManyToOneTests; | |||
There was a problem hiding this comment.
Behavioral regression coverage removed
This deleted test exercised observable public behavior: complete relationship population when an internal lookup crosses the configured query-value boundary. It did not inspect chunk sizes or internal calls. Removing the only coverage of this boundary across relationship types and directions allows the exact failure introduced here to pass while ordinary relationship tests remain green. Retain a behavior-focused version and remove only the redundant direct-query exception assertion.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/e2e/Adapter/Scopes/RelationshipTests.php
Line: 26
Comment:
**Behavioral regression coverage removed**
This deleted test exercised observable public behavior: complete relationship population when an internal lookup crosses the configured query-value boundary. It did not inspect chunk sizes or internal calls. Removing the only coverage of this boundary across relationship types and directions allows the exact failure introduced here to pass while ordinary relationship tests remain green. Retain a behavior-focused version and remove only the redundant direct-query exception assertion.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
No description provided.