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 semantics of bounded numeric operators from "clamp to the limit value when an operation would exceed it" to "leave the stored value unchanged when an operation would cross the configured min/max." The change applies consistently across MariaDB, Postgres, SQLite, MySQL, MongoDB, Memory, and Redis adapters. Additionally, array operator size enforcement is lifted out of the individual SQL adapters into the shared
Confidence Score: 5/5The PR is safe to merge. All seven adapters now apply bounded operators consistently, and the new POWER guards prevent hard database errors for undefined inputs across every engine. The odd-integer exponent path correctly lets negative-base results pass through to POWER() rather than being blocked by the ABS log comparison; the leave-unchanged guard arithmetic is equivalent across adapters (with Postgres using arbitrary-precision NUMERIC to sidestep overflow where MariaDB uses division); the validator-layer array size cap covers all adapters identically; and the new tests exercise inclusive bounds, per-column and per-row isolation, shrink-applies on already-over-limit bases, and each mathematically undefined power combination. No files require special attention. Important Files Changed
Reviews (14): Last reviewed commit: "Address greptile review" | 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 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 docstrings
🧪 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
| if (\floor($exponent) != $exponent) { | ||
| // A negative base to a fractional exponent is not a real number. | ||
| $whens[] = "WHEN {$col} < 0 THEN {$col}"; | ||
| } | ||
| // Cap by magnitude via logarithms so POWER() never runs on a value that would | ||
| // overflow (base^exp > max <=> exp * LOG(base) > LOG(max)). | ||
| if ($oddInteger) { | ||
| // An odd exponent keeps a negative base negative, and a negative result is | ||
| // always within a positive max, so only cap positive bases; negative bases | ||
| // fall through to POWER() and their (negative) result is applied. | ||
| $whens[] = "WHEN {$col} > 0 AND :$bindKey * LOG({$col}) > LOG(:$maxKey) THEN {$col}"; | ||
| } else { | ||
| // Otherwise the result is non-negative, so its magnitude equals its value — | ||
| // cap either sign. ABS() keeps LOG() defined for a negative even-power base. | ||
| $whens[] = "WHEN {$col} <> 0 AND :$bindKey * LOG(ABS({$col})) > LOG(:$maxKey) THEN {$col}"; | ||
| } | ||
|
|
||
| $whenSql = \implode(' ', $whens); | ||
| return "{$quotedColumn} = CASE {$whenSql} ELSE POWER({$col}, :$bindKey) END"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether tests cover bounded zero-to-zero power with max < 1.
rg -n "Operator::power\(0|bounded_power|0\.5" tests srcRepository: utopia-php/database
Length of output: 7762
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the MariaDB power implementation around the cited lines.
file="src/Database/Adapter/MariaDB.php"
wc -l "$file"
sed -n '2000,2085p' "$file"
# Inspect operator tests around power/bounded power cases.
testfile="tests/e2e/Adapter/Scopes/OperatorTests.php"
wc -l "$testfile"
sed -n '1720,1775p' "$testfile"
sed -n '4035,4075p' "$testfile"
sed -n '1415,1475p' "$testfile"Repository: utopia-php/database
Length of output: 12896
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any bounded power test using exponent 0, and inspect the createAttribute max constraint semantics.
rg -n "Operator::power\(0,|power\(0, [^)]*\)|Operator::power\(0\)" tests/e2e/Adapter/Scopes/OperatorTests.php tests/unit/OperatorTest.php src/Database/Adapter/MariaDB.php
# Inspect the operator constructor/shape if needed to understand default arguments.
rg -n "function power|TYPE_POWER|power\(" src tests/unit tests/e2e -g '!vendor'Repository: utopia-php/database
Length of output: 6377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/Database/Validator/Operator.php"
wc -l "$file"
sed -n '160,240p' "$file"Repository: utopia-php/database
Length of output: 4822
Handle 0^0 in the bounded power path (src/Database/Adapter/MariaDB.php:2039-2057).
Operator::power(0, $max) falls through to POWER(0, 0) for a base of 0, so when $max < 1 it writes 1 instead of keeping the existing value. Add a zero-exponent guard before the log-based check.
🤖 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` around lines 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.
1 participant
Summary by CodeRabbit
arrayFilterconditions.