Conversation
…BigInt method - Added abstract method getLimitForBigInt to Adapter class. - Implemented getLimitForBigInt in Mongo, Pool, and SQL classes. - Updated MariaDB and Postgres adapters to handle VAR_BIGINT type. - Enhanced error messages for unknown types to include VAR_BIGINT.
- Introduced VAR_BIGINT constant in Database class. - Updated Attribute class to include maxBigIntLength parameter and handle it in validation. - Ensured backwards compatibility for existing validator construction. - Enhanced validation logic to enforce size limits for VAR_BIGINT type.
- Introduced testCreateAttributesBigIntSizeLimit to verify exception handling for oversized BIGINT attributes. - Added testCreateDocumentWithBigIntType to ensure correct document creation and retrieval with BIGINT attributes. - Implemented testBigIntSizeTooLarge and testUnsignedBigIntSizeLimit to validate size constraints in the Attribute validator.
|
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:
📝 WalkthroughWalkthroughAdds VAR_BIGINT support across the database layer: new adapter accessor, SQL/MariaDB/Postgres and Mongo mappings, validator updates for bigint sizing and validation, Database constant and attribute wiring, Pool delegation, and new unit and e2e tests for bigint behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
Greptile SummaryThis PR introduces a first-class Confidence Score: 4/5Not safe to merge — two P1 runtime defects will crash any code path that calls getLimitForBigInt() or sets a default on a bigint attribute. Two P1 bugs are present: (1) the integer-overflow in getLimitForBigInt() causes a TypeError at runtime in PHP 8 for both the SQL and Mongo adapters; (2) validateDefaultTypes() is missing the VAR_BIGINT case, preventing bigint attributes from having a default value. Both must be fixed before merging. src/Database/Adapter/SQL.php and src/Database/Adapter/Mongo.php (integer overflow), src/Database/Validator/Attribute.php (missing VAR_BIGINT case in validateDefaultTypes). Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/Database/Adapter/SQL.php
Outdated
| // 2^64 - 1 | ||
| // 18446744073709551615 is the maximum value for a 64-bit unsigned integer | ||
| // 9223372036854775807 is the maximum value for a 64-bit signed integer | ||
| return 18446744073709551615; |
There was a problem hiding this comment.
getLimitForBigInt() will throw TypeError at runtime
The literal 18446744073709551615 (2^64 − 1) exceeds PHP_INT_MAX (9223372036854775807). PHP silently promotes it to a float at parse time, and returning a float from a function declared int causes a TypeError in PHP 8. This means every call to getLimitForBigInt() — including the validator and the e2e test — will throw a TypeError at runtime.
The same overflow is present in Mongo.php at line 3017.
The project already defines Database::MAX_BIG_INT = PHP_INT_MAX (the signed 64-bit maximum). The safest representable value is PHP_INT_MAX.
| return 18446744073709551615; | |
| return PHP_INT_MAX; |
There was a problem hiding this comment.
but isn't this will cause issues? cause unsinged big int is of size 18446744073709551615
There was a problem hiding this comment.
You're right that it creates a limitation, but it's unavoidable in PHP.
The core problem: PHP integers are always signed. On 64-bit systems, PHP can only represent integers from -2^63 to 2^63 - 1 (PHP_INT_MAX = 9223372036854775807). Any integer literal larger than that becomes a float at parse time, and returning a float from a function declared int throws a TypeError in PHP 8+.
The tradeoff:
- Current code: Returns
18446744073709551615(becomes a float) → crashes with TypeError - Using PHP_INT_MAX: Limits range to signed 64-bit → works, but can't use full unsigned range
Practical impact:
If you store values > PHP_INT_MAX in the database, PHP code can't manipulate them as integers anyway (they'll be floats, which lose precision at that scale). So limiting validation to PHP_INT_MAX matches what PHP can actually handle.
If you need the full unsigned range:
- Change return type to
string:public function getLimitForBigInt(): stringand return"18446744073709551615" - Update validators to use
bccomp()for comparison instead of numeric operators - Document that BIGINT values must be handled as strings in PHP
What's your preference—limit to PHP_INT_MAX (simpler, matches PHP's integer type) or support full range with strings (more complex, requires string handling)?
src/Database/Adapter/Mongo.php
Outdated
| public function getLimitForBigInt(): int | ||
| { | ||
| // Mongo does not handle integers directly, so using MariaDB limit for now | ||
| return 18446744073709551615; |
There was a problem hiding this comment.
TypeError as in SQL.php
18446744073709551615 is larger than PHP_INT_MAX and will be promoted to a float by PHP. Returning it from an int-typed function throws a TypeError in PHP 8.
| return 18446744073709551615; | |
| return PHP_INT_MAX; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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/Attribute.php (1)
521-545:⚠️ Potential issue | 🟠 MajorMissing
VAR_BIGINTcase invalidateDefaultTypes()will cause default value validation to fail.The
validateDefaultTypes()method doesn't handleDatabase::VAR_BIGINT. When a bigint attribute has a default value set, it will fall through to thedefaultcase and throw an "Unknown attribute type" exception.Since bigint should support integer default values (same as
VAR_INTEGER), add a case for it.🐛 Proposed fix
case Database::VAR_INTEGER: + case Database::VAR_BIGINT: case Database::VAR_FLOAT: case Database::VAR_BOOLEAN: if ($type !== $defaultType) {Note: This assumes bigint default values should be validated as PHP integers. However, since bigint can exceed PHP's
PHP_INT_MAXon 32-bit systems, you may want to also accept string representations. Verify the intended behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Validator/Attribute.php` around lines 521 - 545, The switch in validateDefaultTypes() is missing Database::VAR_BIGINT so bigint defaults fall through to the default error; add Database::VAR_BIGINT to the same case group that contains Database::VAR_INTEGER (i.e., the branch that validates numeric/boolean defaults) so bigint default values are validated like integers; ensure the added case label appears alongside Database::VAR_INTEGER/VAR_FLOAT/VAR_BOOLEAN in Attribute::validateDefaultTypes().
🧹 Nitpick comments (2)
src/Database/Adapter/Postgres.php (1)
2001-2001: Keep the supported-types exception list centralized.The supported-type list in this exception is still hardcoded and easy to desync from switch cases. Consider deriving it from a shared constant/list to avoid future drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Adapter/Postgres.php` at line 2001, The throw in Postgres (the DatabaseException with the long hardcoded VAR_* list) must use a single authoritative source of supported types instead of duplicating them; add or use an existing Database::SUPPORTED_TYPES (or Database::getSupportedTypes()) constant/static method that returns the array of supported VAR_* values, and change the exception construction in Postgres to build the message from that array (e.g., implode(', ', Database::SUPPORTED_TYPES)) so the list is derived from the shared constant; update the throw site that currently references DatabaseException and the VAR_* names to use that shared symbol.src/Database/Validator/Attribute.php (1)
431-451: Consider addingVAR_BIGINTto the supported types list for accurate error messages.When an unknown type is passed, the error message lists supported types but omits
VAR_BIGINT. While this won't cause functional issues (since BIGINT is handled above), adding it would make error messages more accurate.♻️ Suggested improvement
default: $supportedTypes = [ Database::VAR_STRING, Database::VAR_VARCHAR, Database::VAR_TEXT, Database::VAR_MEDIUMTEXT, Database::VAR_LONGTEXT, Database::VAR_INTEGER, + Database::VAR_BIGINT, Database::VAR_FLOAT, Database::VAR_BOOLEAN, Database::VAR_DATETIME, Database::VAR_RELATIONSHIP ];The same addition should be made in
validateDefaultTypes()around line 554.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Validator/Attribute.php` around lines 431 - 451, The supported-types array built in Attribute (the $supportedTypes list in the code path that checks supportForVectors/supportForSpatialAttributes/supportForObject) is missing Database::VAR_BIGINT, so update that array to include Database::VAR_BIGINT; also make the same addition in the validateDefaultTypes() method where a similar $supportedTypes list is constructed so error messages consistently include VAR_BIGINT. Ensure you reference Database::VAR_BIGINT when adding it to both $supportedTypes arrays.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Database/Adapter/MariaDB.php`:
- Around line 1728-1734: VAR_INTEGER was changed to map to INT causing
overflow/regression for callers expecting bigint behavior; revert or preserve
the legacy mapping by making Database::VAR_INTEGER return BIGINT semantics in
MariaDB.php (where the switch handles Database::VAR_INTEGER) or else migrate all
callers to use Database::VAR_BIGINT first. Update the case for
Database::VAR_INTEGER in the MariaDB Adapter so it produces the same BIGINT (and
UNSIGNED handling) as the Database::VAR_BIGINT case (or add a clear migration
TODO and failing assertion in callers like the tests referenced) to avoid
changing runtime column size semantics.
In `@src/Database/Adapter/Mongo.php`:
- Around line 3014-3018: The method getLimitForBigInt currently returns a
numeric literal that exceeds PHP_INT_MAX which breaks the declared return type;
update getLimitForBigInt to return the existing constant Database::MAX_BIG_INT
(or cast that constant to int if necessary) instead of the large literal
18446744073709551615 so the return value is a valid int and consistent with the
rest of the codebase.
- Line 2215: The BSON type mapping for BIGINT in the Database::VAR_BIGINT entry
is incorrect: change its value from 'int' to 'long' so that
partialFilterExpression and sparse/partial indexes use MongoDB 64-bit long type
instead of 32-bit int; locate the mapping array (the place that defines
Database::VAR_BIGINT => 'int') in the Mongo adapter (class/method that builds
type mappings or partialFilterExpression) and update that entry to 'long' so
BIGINT documents are correctly matched by $type checks.
In `@src/Database/Adapter/SQL.php`:
- Around line 904-909: The method getLimitForBigInt currently returns the
unsigned 64-bit literal 18446744073709551615 which exceeds PHP's signed 64-bit
int range and becomes a float; change the return to the signed 64-bit maximum
9223372036854775807 (as an int) so the return value matches the declared int
return type and the tests (DocumentTests.php) expectations; update the return
expression in getLimitForBigInt to use 9223372036854775807 (or cast to int if
needed) to ensure phpstan type checks pass.
In `@src/Database/Database.php`:
- Line 45: You added the new constant Database::VAR_BIGINT but the update and
default-validation code still treat 'bigint' as unknown; update the
type-handling in updateAttribute() and validateDefaultTypes() to recognize
self::VAR_BIGINT (or 'bigint') where other integer-like types are handled (e.g.,
the switch/case or type-to-validator map), and ensure the default/type coercion
paths treat bigint the same as other integer types so updates and default
validation accept bigint attributes.
---
Outside diff comments:
In `@src/Database/Validator/Attribute.php`:
- Around line 521-545: The switch in validateDefaultTypes() is missing
Database::VAR_BIGINT so bigint defaults fall through to the default error; add
Database::VAR_BIGINT to the same case group that contains Database::VAR_INTEGER
(i.e., the branch that validates numeric/boolean defaults) so bigint default
values are validated like integers; ensure the added case label appears
alongside Database::VAR_INTEGER/VAR_FLOAT/VAR_BOOLEAN in
Attribute::validateDefaultTypes().
---
Nitpick comments:
In `@src/Database/Adapter/Postgres.php`:
- Line 2001: The throw in Postgres (the DatabaseException with the long
hardcoded VAR_* list) must use a single authoritative source of supported types
instead of duplicating them; add or use an existing Database::SUPPORTED_TYPES
(or Database::getSupportedTypes()) constant/static method that returns the array
of supported VAR_* values, and change the exception construction in Postgres to
build the message from that array (e.g., implode(', ',
Database::SUPPORTED_TYPES)) so the list is derived from the shared constant;
update the throw site that currently references DatabaseException and the VAR_*
names to use that shared symbol.
In `@src/Database/Validator/Attribute.php`:
- Around line 431-451: The supported-types array built in Attribute (the
$supportedTypes list in the code path that checks
supportForVectors/supportForSpatialAttributes/supportForObject) is missing
Database::VAR_BIGINT, so update that array to include Database::VAR_BIGINT; also
make the same addition in the validateDefaultTypes() method where a similar
$supportedTypes list is constructed so error messages consistently include
VAR_BIGINT. Ensure you reference Database::VAR_BIGINT when adding it to both
$supportedTypes arrays.
🪄 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: 87c67b39-b15b-4568-84ae-ed02167caa45
📒 Files selected for processing (11)
src/Database/Adapter.phpsrc/Database/Adapter/MariaDB.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/Pool.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/SQL.phpsrc/Database/Database.phpsrc/Database/Validator/Attribute.phptests/e2e/Adapter/Scopes/AttributeTests.phptests/e2e/Adapter/Scopes/DocumentTests.phptests/unit/Validator/AttributeTest.php
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Database/Adapter/SQL.php`:
- Around line 899-910: Update the PHPDoc for the getLimitForBigInt method to
match its declared return type string: replace the incorrect "@return float|int"
with "@return string" in the PHPDoc block for the getLimitForBigInt() method so
static analysis (PHPStan) stops flagging the mismatch.
🪄 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: 7545bd28-951f-49ea-b69e-8055e16fd3db
📒 Files selected for processing (6)
Dockerfilesrc/Database/Adapter.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/Pool.phpsrc/Database/Adapter/SQL.phpsrc/Database/Validator/Attribute.php
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Database/Adapter/Pool.php
- src/Database/Validator/Attribute.php
- src/Database/Adapter/Mongo.php
…L adapters - Adjusted the getLimitForBigInt method in both Mongo and SQL classes to return 4294967295, reflecting the maximum value for a 32-bit unsigned integer. - Updated comments to clarify the handling of integer limits in PHP.
ArnabChatterjee20k
force-pushed
the
big-init
branch
from
8b93b40 to
bd14576
Compare
- Updated MariaDB adapter to return 'BIGINT' for sizes greater than or equal to 8 bytes. - Modified SQL adapter to correctly calculate total size for VAR_INTEGER based on the attribute size, distinguishing between INT and BIGINT.
src/Database/Adapter/SQL.php
Outdated
| // 2^64 - 1 | ||
| // 18446744073709551615 is the maximum value for a 64-bit unsigned integer | ||
| // 9223372036854775807 is the maximum value for a 64-bit signed integer | ||
| // in php we can't represent 64-bit integer, so greater than 4294967295 will be treated as bigint |
There was a problem hiding this comment.
@abnegate in php no option for representing a bigint in form of int so having 18446744073709551615 we need to consider string for precision but we can't store the size incoming from user inside the metadata as it will be string and we have to start taking input as string
Shall we go forward with string ? or BIGINT will be marking in the table only?
- Included VAR_BIGINT in the list of attribute types in the Database class. - Ensured consistency in handling BIGINT attributes across the database implementation.
- Added handling for VAR_BIGINT in the size limit validation logic. - Implemented exception throwing for oversized BIGINT attributes to ensure proper error handling.
- Updated validation logic to correctly determine bit size for VAR_BIGINT and VAR_INTEGER based on attribute size. - Ensured consistent handling of signed and unsigned integers across both validators.
- Updated Postgres adapter to return 'BIGINT' for sizes greater than or equal to 8 bytes in the type determination logic for VAR_INTEGER. - Ensured consistent handling of integer types across the database implementation.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Database/Database.php (1)
2586-2593:⚠️ Potential issue | 🟠 Major
bigintdefaults will always fail this type check.PHP reports integer values as
integer, notbigint, soif ($type !== $defaultType)rejects every non-null bigint default. That breaks bigint defaults in attribute create/update flows.💡 Minimal fix
case self::VAR_INTEGER: case self::VAR_BIGINT: + if ($defaultType !== 'integer') { + throw new DatabaseException('Default value ' . $default . ' does not match given type ' . $type); + } + break; case self::VAR_FLOAT: case self::VAR_BOOLEAN: if ($type !== $defaultType) { throw new DatabaseException('Default value ' . $default . ' does not match given type ' . $type); } break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Database.php` around lines 2586 - 2593, The type check in the block handling self::VAR_INTEGER, self::VAR_BIGINT, self::VAR_FLOAT, self::VAR_BOOLEAN incorrectly rejects bigint defaults because PHP reports them as "integer" (so $defaultType != $type); update the condition in the switch branch that throws DatabaseException to accept an integer-typed default when the target $type is self::VAR_BIGINT (i.e., treat $defaultType === 'integer' as valid for self::VAR_BIGINT) while preserving the existing strict check for other types; locate and modify the comparison around $type, $defaultType in the method where the switch uses constants VAR_INTEGER/VAR_BIGINT/VAR_FLOAT/VAR_BOOLEAN and DatabaseException is thrown.
♻️ Duplicate comments (1)
src/Database/Adapter/Mongo.php (1)
3014-3018:⚠️ Potential issue | 🟠 MajorBIGINT limit is still capped to INT-level range.
Line 3017 returns
4294967295, which matchesgetLimitForInt()and removes practical headroom for BIGINT in adapter-level size-limit checks.🔧 Proposed fix
public function getLimitForBigInt(): int { - // Mongo does not handle integers directly, so using MariaDB limit for now - return 4294967295; + return Database::MAX_BIG_INT; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Adapter/Mongo.php` around lines 3014 - 3018, getLimitForBigInt() currently returns 4294967295 (same as getLimitForInt()), removing HEADROOM for BIGINT checks; change getLimitForBigInt() to return the appropriate 64-bit BIGINT max (use 9223372036854775807 to fit the method's int return type on 64-bit builds) so adapter-level size-limit checks reflect BIGINT capacity rather than INT capacity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Database/Adapter/SQL.php`:
- Around line 904-911: The getLimitForBigInt() currently returns 4294967295
(same as getLimitForInt()), so update it to return the correct BIGINT ceiling
(use 9223372036854775807 for signed 64-bit) and/or fall back to PHP_INT_MAX when
appropriate; modify getLimitForBigInt() to return 9223372036854775807 (or
PHP_INT_MAX on platforms where that better represents the native integer max) so
BIGINT checks differ from getLimitForInt().
In `@src/Database/Database.php`:
- Line 45: The atomic counter helpers increaseDocumentAttribute() and
decreaseDocumentAttribute() reject schema-backed bigint attributes because the
whitelist only permits VAR_INTEGER and VAR_FLOAT; update both functions to also
allow the class constant VAR_BIGINT (public const VAR_BIGINT) alongside
VAR_INTEGER and VAR_FLOAT so that schema attributes typed as bigint do not throw
the TypeException—locate the type-check/whitelist logic in
increaseDocumentAttribute() and decreaseDocumentAttribute() and add
self::VAR_BIGINT to the allowed types.
---
Outside diff comments:
In `@src/Database/Database.php`:
- Around line 2586-2593: The type check in the block handling self::VAR_INTEGER,
self::VAR_BIGINT, self::VAR_FLOAT, self::VAR_BOOLEAN incorrectly rejects bigint
defaults because PHP reports them as "integer" (so $defaultType != $type);
update the condition in the switch branch that throws DatabaseException to
accept an integer-typed default when the target $type is self::VAR_BIGINT (i.e.,
treat $defaultType === 'integer' as valid for self::VAR_BIGINT) while preserving
the existing strict check for other types; locate and modify the comparison
around $type, $defaultType in the method where the switch uses constants
VAR_INTEGER/VAR_BIGINT/VAR_FLOAT/VAR_BOOLEAN and DatabaseException is thrown.
---
Duplicate comments:
In `@src/Database/Adapter/Mongo.php`:
- Around line 3014-3018: getLimitForBigInt() currently returns 4294967295 (same
as getLimitForInt()), removing HEADROOM for BIGINT checks; change
getLimitForBigInt() to return the appropriate 64-bit BIGINT max (use
9223372036854775807 to fit the method's int return type on 64-bit builds) so
adapter-level size-limit checks reflect BIGINT capacity rather than INT
capacity.
🪄 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: f6553440-657b-4297-a48a-4c928fd82932
📒 Files selected for processing (7)
src/Database/Adapter/MariaDB.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/SQL.phpsrc/Database/Database.phpsrc/Database/Validator/Query/Filter.phpsrc/Database/Validator/Structure.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Database/Adapter/Postgres.php
- src/Database/Adapter/MariaDB.php
| $attributes = [[ | ||
| '$id' => 'foo', | ||
| 'type' => Database::VAR_BIGINT, | ||
| 'size' => (int)$limit + 1, |
There was a problem hiding this comment.
I think we can ignore size altogether for big int, doesn't make much sense. We only need to validate it's a whole number
- Eliminated size limit checks for VAR_BIGINT in the Database and Attribute classes to allow larger values. - Updated Mongo and SQL adapters to return a constant for BIGINT limits, ensuring consistent handling across implementations. - Refactored tests to reflect the removal of size constraints and validate successful attribute creation without exceptions.
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/Validator/Attribute.php (1)
527-534:⚠️ Potential issue | 🔴 CriticalMissing
VAR_BIGINTcase invalidateDefaultTypes()will reject valid bigint defaults.The
validateDefaultTypes()method does not handleDatabase::VAR_BIGINT. If a bigint attribute is created with a default value, validation will fall through to thedefaultcase (line 548) and throw "Unknown attribute type", even though the type is valid.🐛 Proposed fix
case Database::VAR_INTEGER: + case Database::VAR_BIGINT: case Database::VAR_FLOAT: case Database::VAR_BOOLEAN: if ($type !== $defaultType) {Also update the
$supportedTypesarray in the same method (lines 549-560):$supportedTypes = [ Database::VAR_STRING, Database::VAR_VARCHAR, Database::VAR_TEXT, Database::VAR_MEDIUMTEXT, Database::VAR_LONGTEXT, Database::VAR_INTEGER, + Database::VAR_BIGINT, Database::VAR_FLOAT, Database::VAR_BOOLEAN, Database::VAR_DATETIME, Database::VAR_RELATIONSHIP ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Validator/Attribute.php` around lines 527 - 534, validateDefaultTypes() currently omits Database::VAR_BIGINT so bigint defaults fall through and trigger the "Unknown attribute type" error; add a case for Database::VAR_BIGINT alongside Database::VAR_INTEGER/VAR_FLOAT/VAR_BOOLEAN in the switch block inside validateDefaultTypes() so bigint default type checks use the same logic, and also include Database::VAR_BIGINT in the $supportedTypes array used by validateDefaultTypes() to avoid rejecting valid bigint attributes (refer to the validateDefaultTypes() method and the $supportedTypes variable).
🧹 Nitpick comments (1)
src/Database/Validator/Attribute.php (1)
347-349: Consider addingVAR_BIGINTto$supportedTypesin the default case for error message completeness.The empty case correctly skips size validation per design. However,
VAR_BIGINTis missing from the$supportedTypesarray at lines 426-446. While the explicit case prevents reaching the default, adding it to$supportedTypesensures accurate error messages if future refactoring changes the switch structure.♻️ Suggested addition
$supportedTypes = [ Database::VAR_STRING, Database::VAR_VARCHAR, Database::VAR_TEXT, Database::VAR_MEDIUMTEXT, Database::VAR_LONGTEXT, Database::VAR_INTEGER, + Database::VAR_BIGINT, Database::VAR_FLOAT, Database::VAR_BOOLEAN, Database::VAR_DATETIME, Database::VAR_RELATIONSHIP ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Validator/Attribute.php` around lines 347 - 349, Add Database::VAR_BIGINT to the $supportedTypes array used in the default-case error message to keep the supported-types list accurate; locate the $supportedTypes definition in the Attribute validator (variable name $supportedTypes) and append Database::VAR_BIGINT so that error messages remain correct if the switch in validateSize (the case Database::VAR_BIGINT) is refactored later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Database/Validator/Attribute.php`:
- Around line 527-534: validateDefaultTypes() currently omits
Database::VAR_BIGINT so bigint defaults fall through and trigger the "Unknown
attribute type" error; add a case for Database::VAR_BIGINT alongside
Database::VAR_INTEGER/VAR_FLOAT/VAR_BOOLEAN in the switch block inside
validateDefaultTypes() so bigint default type checks use the same logic, and
also include Database::VAR_BIGINT in the $supportedTypes array used by
validateDefaultTypes() to avoid rejecting valid bigint attributes (refer to the
validateDefaultTypes() method and the $supportedTypes variable).
---
Nitpick comments:
In `@src/Database/Validator/Attribute.php`:
- Around line 347-349: Add Database::VAR_BIGINT to the $supportedTypes array
used in the default-case error message to keep the supported-types list
accurate; locate the $supportedTypes definition in the Attribute validator
(variable name $supportedTypes) and append Database::VAR_BIGINT so that error
messages remain correct if the switch in validateSize (the case
Database::VAR_BIGINT) is refactored later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 781931a2-1bf3-4b5b-a684-47ed567802ae
📒 Files selected for processing (6)
src/Database/Adapter/Mongo.phpsrc/Database/Adapter/SQL.phpsrc/Database/Database.phpsrc/Database/Validator/Attribute.phptests/e2e/Adapter/Scopes/AttributeTests.phptests/unit/Validator/AttributeTest.php
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Database/Adapter/SQL.php
- tests/unit/Validator/AttributeTest.php
- src/Database/Adapter/Mongo.php
- src/Database/Database.php
- Included VAR_BIGINT in the whitelist for attribute types in the Database class. - Implemented a new test to validate the creation, updating, and atomic operations for BIGINT attributes in the DocumentTests. - Ensured that the new tests confirm correct behavior for incrementing and decrementing BIGINT values.
2 participants
Summary by CodeRabbit