Mongo operators#907
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe MongoDB adapter gains aggregation pipeline-based operator support. Casting guards skip ChangesMongoDB Operator Pipeline Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
Greptile SummaryThis PR wires up Operator-based aggregation-pipeline updates for MongoDB, enabling atomic field expressions (increment, array mutations, date arithmetic, etc.) to be applied during
Confidence Score: 3/5The core pipeline update path is well-constructed, but the ON-DUPLICATE-KEY upsert branch does not handle Operator instances, producing driver errors whenever operators are combined with that code path — a gap that now matters because the adapter advertises operator support. The operator translation logic in getOperatorExpression and buildOperatorPipeline is thorough and the two new low-level command helpers correctly avoid the toObject corruption. However, the upsertDocuments branch guarded by !empty($attribute) never routes through buildOperatorPipeline: any Operator in $record (whether the designated attribute or another field) is forwarded raw to MongoDB's $inc or $set, causing an immediate error. This gap affects a real code path that callers can now reach — two of the three callers handle it, but this one does not. src/Database/Adapter/Mongo.php — specifically the !empty($attribute) branch inside upsertDocuments Important Files Changed
Reviews (3): Last reviewed commit: "Greptile address" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Database/Adapter/Mongo.php`:
- Around line 1447-1450: Operator values in Mongo::buildOperatorPipeline() are
currently skipped entirely, which leaves literal operands uncast and can pass
raw strings into $literal for typed attributes. Update the operator-handling
path in Mongo so the field expression stays untouched, but the operands used by
buildOperatorPipeline() are cast to the target attribute type before the
pipeline is assembled; also make sure the same casting behavior is applied in
the other operator-related blocks referenced by the review so datetime arrays
and similar typed operands are encoded as Mongo UTCDateTime values instead of
strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10051415-6227-4a2b-ade4-7162b0d73df4
📒 Files selected for processing (1)
src/Database/Adapter/Mongo.php
| // Operators are resolved by the database (aggregation pipeline); skip casting | ||
| if (Operator::isOperator($value)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Cast operator literal operands before building the pipeline.
Line 1448 skips casting the whole Operator, and buildOperatorPipeline() later writes operand values through $literal. Since Database::encode() also skips Operator values, typed operands such as arrayAppend()/arrayInsert() on datetime arrays can be stored as raw strings instead of Mongo UTCDateTime values. Keep the field expression uncast, but cast operator operand literals according to the target attribute before translating them.
Also applies to: 1744-1765, 1921-1935
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Database/Adapter/Mongo.php` around lines 1447 - 1450, Operator values in
Mongo::buildOperatorPipeline() are currently skipped entirely, which leaves
literal operands uncast and can pass raw strings into $literal for typed
attributes. Update the operator-handling path in Mongo so the field expression
stays untouched, but the operands used by buildOperatorPipeline() are cast to
the target attribute type before the pipeline is assembled; also make sure the
same casting behavior is applied in the other operator-related blocks referenced
by the review so datetime arrays and similar typed operands are encoded as Mongo
UTCDateTime values instead of strings.
There was a problem hiding this comment.
@fogelito Sounds good — thanks for splitting that follow-up into #908. I’ll treat this one as deferred to that PR.
🧠 Learnings used
Learnt from: abnegate
Repo: utopia-php/database PR: 860
File: src/Database/Adapter/Memory.php:1279-1284
Timestamp: 2026-04-29T03:59:51.515Z
Learning: For all database adapters in src/Database/Adapter (including Memory.php), ensure getMaxIndexLength() is expressed in bytes (not kilobytes) and remains consistent with the expectations of the e2e scope tests under tests/e2e/Adapter/Scopes/. The IndexTests/AttributeTests derive attribute sizes and error messages from getMaxIndexLength() and assume attributes are created using values like getMaxIndexLength() + 300 and getMaxIndexLength() - 68. If you change getMaxIndexLength() (e.g., from 1024 to another value), update the scope tests and any adapter string-limit behavior accordingly to prevent breaking size/error-message assertions.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes