Operators limits#911
Conversation
…max-param2 # Conflicts: # src/Database/Adapter/MariaDB.php
![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/MariaDB.php`:
- Line 2036: The MariaDB adapter’s POWER(COALESCE(...), :$bindKey) branch can
still evaluate to NULL for invalid exponent/base combinations, causing the
update path to overwrite the column with NULL. Update the MariaDB adapter logic
around this POWER expression to guard against these cases by preserving the
existing column value when the power result would be NULL, and add coverage for
zero and negative-base/fractional-exponent scenarios in the same update flow.
🪄 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: 996433f1-7d7f-4130-9836-989ff5446fb6
📒 Files selected for processing (10)
src/Database/Adapter/MariaDB.phpsrc/Database/Adapter/Memory.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/MySQL.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/Redis.phpsrc/Database/Adapter/SQL.phpsrc/Database/Adapter/SQLite.phpsrc/Database/Operator.phptests/e2e/Adapter/Scopes/OperatorTests.php
✅ Files skipped from review due to trivial changes (1)
- src/Database/Adapter/SQL.php
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Database/Adapter/Memory.php
- src/Database/Adapter/Redis.php
- src/Database/Adapter/Mongo.php
- src/Database/Adapter/SQLite.php
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/MariaDB.php`:
- Line 2036: The MariaDB adapter’s POWER(COALESCE(...), :$bindKey) branch can
still evaluate to NULL for invalid exponent/base combinations, causing the
update path to overwrite the column with NULL. Update the MariaDB adapter logic
around this POWER expression to guard against these cases by preserving the
existing column value when the power result would be NULL, and add coverage for
zero and negative-base/fractional-exponent scenarios in the same update flow.
🪄 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: 996433f1-7d7f-4130-9836-989ff5446fb6
📒 Files selected for processing (10)
src/Database/Adapter/MariaDB.phpsrc/Database/Adapter/Memory.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/MySQL.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/Redis.phpsrc/Database/Adapter/SQL.phpsrc/Database/Adapter/SQLite.phpsrc/Database/Operator.phptests/e2e/Adapter/Scopes/OperatorTests.php
✅ Files skipped from review due to trivial changes (1)
- src/Database/Adapter/SQL.php
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Database/Adapter/Memory.php
- src/Database/Adapter/Redis.php
- src/Database/Adapter/Mongo.php
- src/Database/Adapter/SQLite.php
🛑 Comments failed to post (1)
src/Database/Adapter/MariaDB.php (1)
2036-2036: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect power-related e2e tests to see base values used against the unbounded branch rg -nP -C4 'Operator::power\(' tests/e2e/Adapter/Scopes/OperatorTests.phpRepository: utopia-php/database
Length of output: 6550
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the MariaDB power handling around the referenced line and nearby branches. ast-grep outline src/Database/Adapter/MariaDB.php --view expanded | sed -n '1,240p' printf '\n---\n' sed -n '2000,2065p' src/Database/Adapter/MariaDB.phpRepository: utopia-php/database
Length of output: 4437
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect power-related operator tests around edge cases and any parity assertions. rg -nC 3 'Operator::power\(|zero base|negative exponent|fractional exponent|undefined|NULL' tests/e2e/Adapter/Scopes/OperatorTests.phpRepository: utopia-php/database
Length of output: 8402
🌐 Web query:
MariaDB POWER negative base fractional exponent NULL documentation💡 Result:
In MariaDB, the POWER(X, Y) function—which is a synonym for POW(X, Y)—behaves in the following ways regarding negative bases, fractional exponents, and NULL inputs: Negative Base with Fractional Exponent: When you raise a negative base to a non-integer (fractional) exponent, the result is mathematically complex (imaginary). Because MariaDB does not support complex numbers in its standard numeric functions, it returns NULL for these operations [1][2]. For example, POWER(-2, 1.5) returns NULL [1]. NULL Inputs: If either the base (X) or the exponent (Y) is NULL, the POWER function returns NULL [1][3][2]. Handling These Cases: To avoid these NULL results, you can implement checks using SQL functions: - To handle negative bases, you can use the ABS function to perform the calculation on the absolute value, or use a CASE statement to validate the inputs before execution [1][2]. - To handle NULL values, you can use COALESCE or IFNULL to provide a default value [1][2]. For example, to safely handle a potentially NULL exponent, you might use: SELECT POWER(10, COALESCE(my_exponent, 0)); [1]
Citations:
- 1: https://runebook.dev/en/docs/mariadb/power/index
- 2: https://runebook.dev/en/docs/mariadb/pow/index
- 3: https://myway-sql.com/mysql_functions/POWER/
🌐 Web query:
MariaDB POWER 0 negative exponent NULL documentation💡 Result:
In MariaDB, the POWER(X, Y) function (which is a synonym for POW(X, Y)) calculates X raised to the power of Y [1][2][3]. Regarding the specific case of POWER(0, negative_exponent), MariaDB typically returns a result based on floating-point arithmetic rather than returning NULL. In mathematical terms, raising zero to a negative power is undefined (division by zero), and MariaDB will generally return NULL or an error/warning depending on the specific environment configuration and version, as it is a domain error [4][3][5]. Documentation and general behavior summary: 1. NULL Handling: If either the base or the exponent is NULL, the POWER function returns NULL [4][6]. 2. Negative Exponents: When the base is a positive non-zero number, negative exponents are handled correctly (e.g., POWER(2, -2) returns 0.25) [2][7]. 3. Base 0 and Negative Exponent: This is a mathematical domain error. While the SQL standard suggests raising an exception for this case, MariaDB's implementation may return NULL for such undefined operations [4][5]. To handle potential issues safely, you can use COALESCE to manage NULL inputs [4] or a CASE statement to validate your inputs before passing them to the POWER function [4][3].
Citations:
- 1: https://mariadb.com/docs/server/reference/sql-functions/numeric-functions/power
- 2: https://mariadb.com/docs/server/reference/sql-functions/numeric-functions/pow
- 3: https://runebook.dev/en/docs/mariadb/pow/index
- 4: https://runebook.dev/en/docs/mariadb/power/index
- 5: https://bugs.mysql.com/bug.php?id=71220
- 6: https://myway-sql.com/mysql_functions/POWER/
- 7: https://www.techonthenet.com/mariadb/functions/power.php
Guard the unbounded power path
POWER(COALESCE({$quotedColumn}, 0), :$bindKey)can still returnNULLfor0^negativeor negative bases with fractional exponents on MariaDB, so this branch can writeNULLback to the column instead of preserving the current value. The current tests only cover positive bases.🤖 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/MariaDB.php` at line 2036, The MariaDB adapter’s POWER(COALESCE(...), :$bindKey) branch can still evaluate to NULL for invalid exponent/base combinations, causing the update path to overwrite the column with NULL. Update the MariaDB adapter logic around this POWER expression to guard against these cases by preserving the existing column value when the power result would be NULL, and add coverage for zero and negative-base/fractional-exponent scenarios in the same update flow.
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/Validator/Operator.php`:
- Around line 153-167: Count the normalized array payload in Operator::parse()
instead of the raw top-level $values array, so nested inputs like a single
wrapped list are still capped correctly. Update the array-operator guard to use
the same normalized count logic and include DatabaseOperator::TYPE_ARRAY_REMOVE
alongside TYPE_ARRAY_APPEND, TYPE_ARRAY_PREPEND, TYPE_ARRAY_INTERSECT, and
TYPE_ARRAY_DIFF so arrayRemove() is limited the same way.
🪄 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: 2f72bd7e-050c-439c-9681-a34b00d96dae
📒 Files selected for processing (7)
src/Database/Adapter/MariaDB.phpsrc/Database/Adapter/MySQL.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/SQLite.phpsrc/Database/Operator.phpsrc/Database/Validator/Operator.phptests/e2e/Adapter/Scopes/OperatorTests.php
💤 Files with no reviewable changes (4)
- src/Database/Adapter/MySQL.php
- src/Database/Adapter/Postgres.php
- src/Database/Adapter/SQLite.php
- src/Database/Adapter/MariaDB.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Database/Operator.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Database/Adapter/Mongo.php (1)
1896-1905: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't short-circuit every bounded power when the base is
<= 1.This turns valid bounded powers into no-ops. For example,
0.5 ^ 2withmax = 1should become0.25, and(-4) ^ 2withmax = 20should become16, but this branch returns the original base in both cases. Only bypass$powfor truly undefined inputs; otherwise compare the computed result against the bound.🤖 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 1896 - 1905, The bounded power logic in Mongo::convertExpression incorrectly short-circuits all cases where the base is <= 1, causing valid computations in the power-handling branch to return the original base instead of the bounded result. Update the condition inside the $cond block in Mongo::convertExpression so it only skips $pow for truly undefined inputs (such as invalid fractional/negative-power cases), and otherwise always compare the computed $expr against $values[1] before deciding whether to cap it or keep it.
♻️ Duplicate comments (1)
src/Database/Validator/Operator.php (1)
155-165: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCap the normalized array payload here too.
count($values)only sees the outer container, so wrapped payloads can still bypass this limit, andTYPE_ARRAY_REMOVEis still excluded even though its validation path already normalizesvalues[0]later in the method. Reuse that normalized shape here and apply the same cap to remove operations as well.Suggested fix
+ $arrayOperatorValues = \is_array($values[0] ?? null) ? $values[0] : $values; if ( \in_array($method, [ DatabaseOperator::TYPE_ARRAY_APPEND, DatabaseOperator::TYPE_ARRAY_PREPEND, DatabaseOperator::TYPE_ARRAY_INTERSECT, DatabaseOperator::TYPE_ARRAY_DIFF, + DatabaseOperator::TYPE_ARRAY_REMOVE, ], true) - && \count($values) > DatabaseOperator::MAX_ARRAY_OPERATOR_SIZE + && \count($arrayOperatorValues) > DatabaseOperator::MAX_ARRAY_OPERATOR_SIZE ) { - $this->message = "Array size " . \count($values) . " exceeds maximum allowed size of " . DatabaseOperator::MAX_ARRAY_OPERATOR_SIZE . " for array operations"; + $this->message = "Array size " . \count($arrayOperatorValues) . " exceeds maximum allowed size of " . DatabaseOperator::MAX_ARRAY_OPERATOR_SIZE . " for array operations"; return false; }🤖 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/Validator/Operator.php` around lines 155 - 165, The array-size guard in Operator::validate currently checks only count($values) and omits TYPE_ARRAY_REMOVE, so wrapped payloads can bypass the limit. Update the validation to use the normalized array shape already used later in the method (including values[0] for wrapped payloads) and apply the same MAX_ARRAY_OPERATOR_SIZE cap to TYPE_ARRAY_REMOVE as well as the other array operators.
🤖 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.
Outside diff comments:
In `@src/Database/Adapter/Mongo.php`:
- Around line 1896-1905: The bounded power logic in Mongo::convertExpression
incorrectly short-circuits all cases where the base is <= 1, causing valid
computations in the power-handling branch to return the original base instead of
the bounded result. Update the condition inside the $cond block in
Mongo::convertExpression so it only skips $pow for truly undefined inputs (such
as invalid fractional/negative-power cases), and otherwise always compare the
computed $expr against $values[1] before deciding whether to cap it or keep it.
---
Duplicate comments:
In `@src/Database/Validator/Operator.php`:
- Around line 155-165: The array-size guard in Operator::validate currently
checks only count($values) and omits TYPE_ARRAY_REMOVE, so wrapped payloads can
bypass the limit. Update the validation to use the normalized array shape
already used later in the method (including values[0] for wrapped payloads) and
apply the same MAX_ARRAY_OPERATOR_SIZE cap to TYPE_ARRAY_REMOVE as well as the
other array operators.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b1f21c55-6eb2-4281-92a9-0363c609ec91
📒 Files selected for processing (6)
src/Database/Adapter/Memory.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/Redis.phpsrc/Database/Validator/Operator.phptests/e2e/Adapter/Scopes/OperatorTests.php
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Database/Adapter/Postgres.php
- src/Database/Adapter/Memory.php
- src/Database/Adapter/Redis.php
- tests/e2e/Adapter/Scopes/OperatorTests.php
Greptile SummaryThis PR changes the bounded numeric operator semantics across all adapters (SQL, Postgres, SQLite, Memory, Redis, Mongo): when an operation would exceed a min/max bound, the stored value is now left unchanged rather than clamped to the limit. It also adds comprehensive edge-case handling for
Confidence Score: 5/5The PR is safe to merge. All previously identified issues with POWER edge cases, overflow guards, and test correctness have been resolved in this revision. Every adapter now handles the "leave unchanged" semantics consistently. The POWER rewrite correctly uses exponent-specific logarithm guards to avoid calling POWER on undefined inputs (zero raised to a negative exponent, negative base with fractional exponent, overflow). Memory/Redis increment/decrement guards correctly allow negative-$by operations without short-circuiting. The Validator centralises size and condition checks so adapters cannot be reached with invalid payloads. The new test suite is thorough: it covers bounded shrink, per-column isolation, per-row batch, inclusive bounds, all POWER edge cases, and array operator limits. No files require special attention. All changed files align with the new "leave unchanged" contract. Important Files Changed
Reviews (16): Last reviewed commit: "Check is operator" | Re-trigger Greptile |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNumeric operator bound handling was changed across MariaDB, Postgres, SQLite, Mongo, Memory, and Redis adapters to return the existing/original value instead of clamping to min/max when a bound would be exceeded, with new power-overflow and undefined-case guards. Array operator size limits were consolidated into a shared ChangesOperator bound and size-limit updates
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
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/MariaDB.php`:
- Around line 2039-2057: The bounded power handling in MariaDBAdapter’s power
update path does not special-case a zero exponent, so `Operator::power(0, $max)`
can still fall through to `POWER(0, 0)` and write `1` when `$max < 1`. Update
the CASE-building logic in the MariaDB `power`/bounded exponent branch to add an
explicit zero-exponent guard before the log-based overflow checks, ensuring the
existing column value is preserved for `0^0` instead of letting `POWER()`
decide.
🪄 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: 1ae16548-2b57-4d8b-8283-f085e148c5f4
📒 Files selected for processing (8)
src/Database/Adapter/MariaDB.phpsrc/Database/Adapter/Memory.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/Redis.phpsrc/Database/Adapter/SQLite.phpsrc/Database/Validator/Operator.phptests/e2e/Adapter/Scopes/OperatorTests.php
🚧 Files skipped from review as they are similar to previous changes (7)
- src/Database/Validator/Operator.php
- src/Database/Adapter/Mongo.php
- src/Database/Adapter/Redis.php
- src/Database/Adapter/Postgres.php
- src/Database/Adapter/Memory.php
- src/Database/Adapter/SQLite.php
- tests/e2e/Adapter/Scopes/OperatorTests.php
1 participant
Summary by CodeRabbit
arrayFilterconditions.