feat(#410): add support for LTREE type.
#411
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
This pull request only add the |
WalkthroughAdds PostgreSQL ltree support: a new Ltree DBAL type and immutable Ltree value object, conversion and validation exceptions, unit and integration tests (including test DB setup), and documentation for Doctrine/Laravel/Symfony integration and migration/indexing guidance. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Type as Ltree DBAL Type
participant VO as Ltree VO
participant DB as Database
rect rgb(232,244,255)
note right of App: PHP -> DB (convertToDatabaseValue)
App->>Type: convertToDatabaseValue(value, platform)
alt value is null
Type-->>App: null
else value is string
Type->>VO: Ltree::fromString(value)
alt invalid format
Type-->>App: throw InvalidLtreeForPHPException
else valid
VO-->>Type: string
Type-->>App: string
end
else value is VO
VO-->>Type: string
Type-->>App: string
else
Type-->>App: throw InvalidLtreeForPHPException
end
end
rect rgb(255,244,240)
note right of App: DB -> PHP (convertToPHPValue)
App->>Type: convertToPHPValue(dbValue, platform)
alt dbValue is null
Type-->>App: null
else dbValue is string
Type->>VO: Ltree::fromString(dbValue)
alt invalid format
Type-->>App: throw InvalidLtreeForDatabaseException
else valid
VO-->>Type: instance
Type-->>App: Ltree VO
end
else
Type-->>App: throw InvalidLtreeForDatabaseException
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
tests/Integration/MartinGeorgiev/TestCase.php (1)
163-190: Registering 'ltree' type is correct; consider centralizing extension setupMapping 'ltree' => Ltree::class looks good. Since the ltree extension is required at DB level, consider centralizing extension creation in this base TestCase (similar to createTestSchema) so any future integration tests using ltree don’t need to duplicate setup.
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (2)
75-81: Optional: Assert immutability when retrieving parentConsider asserting that the original ltree remains unchanged after getParent(), to guarantee immutability of the value object.
Example:
$original = new Ltree(['a', 'b', 'c']); $parent = $original->getParent(); self::assertSame(['a', 'b', 'c'], $original->getBranch());
96-102: Nit: Clarify the comment and assert exception messageThe comment mentions “invalid type” but the failure is about an invalid value (empty string). Consider updating the comment and also asserting on the exception message to pin behavior.
Suggested tweak:
-// @phpstan-ignore-next-line argument.type - Testing invalid type handling +// Testing invalid value handling (empty label) $ltree->createLeaf('');tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (3)
140-151: Docblock suggests nullable Ltree but provider never returns nullEither drop the nullable marker in the docblock or add a null case to validate null round-trip for ltree.
Option A (simplify docblock):
- * @return array<string, array{string, string, ?LtreeValueObject}> + * @return array<string, array{string, string, LtreeValueObject}>Option B (add null case):
return [ + 'ltree null' => ['ltree', 'LTREE', null],
134-138: Type-hint the parameter to LtreeValueObject for consistencyOther type-specific tests (e.g., point) type-hint their third parameter. Consider mirroring that here for consistency.
- public function test_ltree_type(string $typeName, string $columnType, mixed $testValue): void + public function test_ltree_type(string $typeName, string $columnType, LtreeValueObject $testValue): void
255-259: Creating the ltree extension is good; consider moving to base TestCaseThis ensures the type and operators are available. If more tests rely on ltree, consider moving this to the Integration TestCase base to DRY setup.
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (2)
82-86: Misleading exception message
assertPostgreSQLPlatform()throws
'LtreeType can only be used with PostgreSQL platform.', but the public class is namedLtree.
Minor, yet this message surfaces in user logs—rename to avoid confusion.- throw new \InvalidArgumentException('LtreeType can only be used with PostgreSQL platform.'); + throw new \InvalidArgumentException('Ltree can only be used with PostgreSQL platform.');
20-25: Suppress unused-parameter warningsStatic-analysis flags
$columnas unused.
Either prefix it with an underscore or add a@psalm-suppress UnusedParam/@phpstan-ignore-next-lineto keep the method noise-free.src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
42-44: Typo in docblock tag
@tthrows→@throws.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php(5 hunks)tests/Integration/MartinGeorgiev/TestCase.php(2 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-26T02:46:12.804Z
Learning: The PR "preserve the type of floats and integers when transforming back and forth between PostgreSQL and PHP" improves type handling by ensuring that integers remain integers, floats remain floats, numeric strings stay as strings, and booleans are properly converted through the transformation process.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: The PostgreSQL-for-doctrine library implements the JSON array element existence check using the `RIGHT_EXISTS_ON_LEFT` DQL function, which maps to PostgreSQL's `?` operator. This is used for checking if a specific value exists in a JSONB array column.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#277
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonValueTest.php:19-35
Timestamp: 2025-03-13T15:00:16.619Z
Learning: The JsonValue implementation in postgres-for-doctrine has documented limitations: it does not support RETURNING type, DEFAULT clause, ON ERROR clause, ON EMPTY clause, and other advanced PostgreSQL JSON_VALUE features due to DQL limitations.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LogTest.php:22-24
Timestamp: 2025-05-23T11:24:46.898Z
Learning: The LOG function in PostgreSQL/Doctrine implementation supports both single-argument form (LOG(x) for base 10 logarithm) and two-argument form (LOG(b, x) for logarithm with specified base).
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: When using operator-like functions in PostgreSQL-for-doctrine, they must be used as boolean functions with `= TRUE` rather than as direct operators (e.g., `RIGHT_EXISTS_ON_LEFT(column, value) = TRUE` instead of `column RIGHT_EXISTS_ON_LEFT value`).
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-04-22T00:03:37.733Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-04-20T11:24:18.300Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: The PostgreSQL-for-doctrine library implements the JSON array element existence check using the `RIGHT_EXISTS_ON_LEFT` DQL function, which maps to PostgreSQL's `?` operator. This is used for checking if a specific value exists in a JSONB array column.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-03-13T15:00:16.619Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#277
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonValueTest.php:19-35
Timestamp: 2025-03-13T15:00:16.619Z
Learning: The JsonValue implementation in postgres-for-doctrine has documented limitations: it does not support RETURNING type, DEFAULT clause, ON ERROR clause, ON EMPTY clause, and other advanced PostgreSQL JSON_VALUE features due to DQL limitations.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-03-26T02:46:12.804Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-26T02:46:12.804Z
Learning: The PR "preserve the type of floats and integers when transforming back and forth between PostgreSQL and PHP" improves type handling by ensuring that integers remain integers, floats remain floats, numeric strings stay as strings, and booleans are properly converted through the transformation process.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
tests/Integration/MartinGeorgiev/TestCase.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: When using operator-like functions in PostgreSQL-for-doctrine, they must be used as boolean functions with `= TRUE` rather than as direct operators (e.g., `RIGHT_EXISTS_ON_LEFT(column, value) = TRUE` instead of `column RIGHT_EXISTS_ON_LEFT value`).
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-05-27T16:10:35.054Z
Learnt from: karpilin
PR: martin-georgiev/postgresql-for-doctrine#386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: When reviewing PostgreSQL formatting function tests, the error handling behavior should match PostgreSQL's actual implementation rather than assuming consistency across all formatting functions, as different functions may handle invalid inputs differently.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
🧬 Code Graph Analysis (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (2)
create(11-14)forInvalidType(16-19)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (2)
create(11-14)forInvalidType(16-19)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php (5)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (1)
InvalidLtreeForDatabaseException(9-25)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (1)
InvalidLtreeForPHPException(9-20)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
Ltree(15-88)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
Ltree(10-130)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (1)
LtreeTest(10-110)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
__construct(17-21)
🪛 PHPMD (2.15.0)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
20-20: Avoid unused parameters such as '$column'. (Unused Code Rules)
(UnusedFormalParameter)
34-34: Avoid unused parameters such as '$platform'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (6)
tests/Integration/MartinGeorgiev/TestCase.php (1)
29-29: LGTM: Import added for new Ltree DBAL typeThe import of MartinGeorgiev\Doctrine\DBAL\Types\Ltree is correct and consistent with the other type imports.
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (2)
65-73: Confirm isRoot() semantics for single-element and empty pathsThis asserts isRoot() is true for both [] and ['a']. Please confirm this matches LtreeInterface semantics and PostgreSQL ltree expectations. If “root” should be only the empty path, adjust accordingly.
Would you like me to scan the value object implementation to verify the intended behavior?
18-23: LGTM: fromString round-tripGood coverage for fromString() and branch round-trip.
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (1)
221-229: LGTM: ltree round-trip assertion hookAdding the ltree branch in assertDatabaseRoundtripEquals keeps the pattern consistent across types.
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
28-31: Return-type does not match the value actually returned
getBindingType()declaresParameterTypeas its return type, yet the method returns the enum caseParameterType::STRING(the case, not an object).
If the project is still on Doctrine DBAL ≤ 3,ParameterTypeis a class with int constants, so this will cause aTypeError.
Even on DBAL 4 (whereParameterTypeis an enum), the return type should beParameterType| int for forward-compatibility with both major versions.- public function getBindingType(): ParameterType - { - return ParameterType::STRING; - } + public function getBindingType(): int + { + return ParameterType::STRING; // enum-safe and legacy-safe + }⛔ Skipped due to learnings
Learnt from: martin-georgiev PR: martin-georgiev/postgresql-for-doctrine#340 File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145 Timestamp: 2025-04-11T11:23:44.192Z Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.Learnt from: martin-georgiev PR: martin-georgiev/postgresql-for-doctrine#0 File: :0-0 Timestamp: 2025-03-26T02:46:12.804Z Learning: The PR "preserve the type of floats and integers when transforming back and forth between PostgreSQL and PHP" improves type handling by ensuring that integers remain integers, floats remain floats, numeric strings stay as strings, and booleans are properly converted through the transformation process.src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
82-85:isRoot()semantics may surpriseTreating both empty branch and single-label branch as root (
count() <= 1) conflates two different concepts.
If a non-empty single label is supposed to behave as a regular node (which can have a parent), you might wantcount($branch) === 0instead.
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php
Show resolved
Hide resolved
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php
Show resolved
Hide resolved
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
Outdated
Show resolved
Hide resolved
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
Outdated
Show resolved
Hide resolved
Contributor
Author
|
I'll work on the failed checks tomorrow. The Ltree value object also needs to inherit JsonSerializable. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (3)
27-32: Use platform in getMappedDatabaseTypes (and avoid mapping on non-PostgreSQL platforms)Returning a mapping only when the platform is PostgreSQL is safer and also resolves the PHPMD unused parameter hint.
Apply this diff:
#[\Override] public function getMappedDatabaseTypes(AbstractPlatform $platform): array { - return [ - 'ltree', - ]; + if ($platform instanceof PostgreSQLPlatform) { + return ['ltree']; + } + + return []; }
18-24: Suppress PHPMD unused parameter warning for $column$column isn't used by design for ltree. Add a suppression to keep static analysis signal clean.
Apply this diff:
- #[\Override] - public function getSQLDeclaration(array $column, AbstractPlatform $platform): string + /** + * @param array<string, mixed> $column + * @SuppressWarnings("PHPMD.UnusedFormalParameter") + */ + #[\Override] + public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
77-79: Nit: Clarify exception message wordingMinor naming consistency: the class is Ltree (a DBAL type). Adjust the message for clarity.
- throw new \InvalidArgumentException('LtreeType can only be used with PostgreSQL platform.'); + throw new \InvalidArgumentException('Ltree DBAL type can only be used with the PostgreSQL platform.');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
🧬 Code Graph Analysis (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (5)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (3)
InvalidLtreeForDatabaseException(9-25)forInvalidFormat(21-24)forInvalidType(16-19)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (2)
InvalidLtreeForPHPException(9-20)forInvalidType(16-19)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (11)
Ltree(10-136)Override(23-27)Override(29-39)Override(41-45)Override(47-61)Override(63-67)Override(69-73)Override(75-79)Override(81-85)Override(87-91)Override(93-104)src/MartinGeorgiev/Doctrine/DBAL/Types/BaseType.php (2)
BaseType(17-52)getSQLDeclaration(24-29)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
Override(24-25)fromString(19-19)
🪛 PHPMD (2.15.0)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
19-19: Avoid unused parameters such as '$column'. (Unused Code Rules)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$platform'. (Unused Code Rules)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PostgreSQL 16 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PHP 8.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: sloth
🔇 Additional comments (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (4)
18-24: PostgreSQL-only guard and straightforward SQL declaration look goodRestricting usage to PostgreSQL and returning the literal 'ltree' is clear and correct for this type.
Also applies to: 75-80
48-57: Conversion to PHP value handles strings well and surfaces format errors properlyCatching the value-object validation exception and rethrowing a domain-specific ConversionException is the right approach. This also supports empty-string ltree values as expected.
59-73: Strict input contract for convertToDatabaseValue is appropriateAccepting only LtreeInterface (or null) keeps DB writes validated and consistent. Casting to string is correct for parameter binding.
18-19: Confirm Doctrine DBAL return‐type compatibilityPHP < 8.3 will silently ignore unknown attributes, so using
#[\Override]under your^8.1constraint is safe. However, narrowing return types to?LtreeValueObjectassumes that the baseDoctrine\DBAL\Types\Typemethods declare a compatible return type (e.g.mixed). Please verify you’re on Doctrine DBAL 3.x (which usesmixedreturn types ongetSQLDeclaration,convertToPHPValue, etc.) by checking your composer.json or running:composer show doctrine/dbalReview the following locations for consistency:
- src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (lines 18–19, 26–27, 39–41, 59–61)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
README.md (1)
61-61: Move ltree out of “Range Types” — it’s not a range.ltree is a hierarchical label path type, not a range. Listing it under “Range Types” can mislead users.
Apply this diff to place ltree in its own category:
- - Numeric ranges (`numrange`, `int4range`, `int8range`) - [ltree](https://www.postgresql.org/docs/current/ltree.html) data type (`ltree`) + - Numeric ranges (`numrange`, `int4range`, `int8range`) + +### Hierarchical Types +- [ltree](https://www.postgresql.org/docs/current/ltree.html) (`ltree`)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
68-76: Optionally accept raw strings in convertToDatabaseValue (aligns with “mapped to string representation”).Issue #410 mentions mapping to a string representation. Allowing strings here reduces friction when users don’t need the ValueObject at write time.
Apply this minimal change:
- if ($value instanceof LtreeInterface) { - return (string) $value; - } + if ($value instanceof LtreeInterface || \is_string($value)) { + return (string) $value; + }If tests deliberately enforce only ValueObject inputs, skip this.
docs/INTEGRATING-WITH-SYMFONY.md (1)
35-36: Document enabling the PostgreSQL ltree extension (required at DB level).Users must ensure the ltree extension exists, otherwise schema operations and queries will fail. Consider adding a short “Enable extension” note with a migration snippet.
Suggested addition to this doc:
# Ensure the ltree extension is enabled (run once, e.g. in a migration) # Doctrine Migrations example (PHP): # $this->addSql('CREATE EXTENSION IF NOT EXISTS ltree');Optionally add an index example:
-- Example GIST index for ltree CREATE INDEX path_gist_idx ON your_table USING GIST (path gist_ltree_ops(siglen = 100));Also applies to: 86-87
docs/INTEGRATING-WITH-DOCTRINE.md (1)
257-258: Add guidance to enable the ltree extension and (optionally) index creation.Without CREATE EXTENSION ltree, the type won’t be usable. Including a brief example improves DX and reduces support issues.
Proposed doc snippet near the mapping section:
$platform->registerDoctrineTypeMapping('tstzrange','tstzrange'); +$em->getConnection()->executeStatement('CREATE EXTENSION IF NOT EXISTS ltree'); + +$em->getConnection()->executeStatement( + 'CREATE INDEX IF NOT EXISTS path_gist_idx ON your_table USING GIST (path gist_ltree_ops(siglen=100))' +); ...If you prefer migrations, replace with a Doctrine Migrations example instead.
docs/INTEGRATING-WITH-LARAVEL.md (1)
347-349: Suggest documenting extension enablement for Laravel users.Many deployments won’t have ltree preinstalled; add a note to enable it via a migration (recommended) rather than at runtime (which may require superuser).
Doc snippet to add:
// Laravel migration example public function up(): void { DB::statement('CREATE EXTENSION IF NOT EXISTS ltree'); // optional index for faster path queries DB::statement('CREATE INDEX IF NOT EXISTS path_gist_idx ON your_table USING GIST (path gist_ltree_ops(siglen=100))'); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(1 hunks)docs/AVAILABLE-TYPES.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(2 hunks)docs/INTEGRATING-WITH-LARAVEL.md(5 hunks)docs/INTEGRATING-WITH-SYMFONY.md(5 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/AVAILABLE-TYPES.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Applied to files:
README.mddocs/INTEGRATING-WITH-DOCTRINE.mddocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
docs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
docs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
docs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PostgreSQL 16 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PHP 8.2
- GitHub Check: sloth
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
🔇 Additional comments (6)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
18-26: #[\Override] requires PHP 8.3 — conflicts with project’s PHP 8.1+ support.Using the built-in Override attribute will fatally error on PHP < 8.3 unless you polyfill a global Override attribute. Either remove the attributes or bump the minimum PHP version.
Apply this diff to keep 8.1 compatibility:
- #[\Override] public function getSQLDeclaration(array $column, AbstractPlatform $platform): string @@ - #[\Override] public function getMappedDatabaseTypes(AbstractPlatform $platform): array @@ - #[\Override] public function convertToPHPValue($value, AbstractPlatform $platform): ?LtreeValueObject @@ - #[\Override] public function convertToDatabaseValue($value, AbstractPlatform $platform): ?stringAlso applies to: 28-36, 43-61, 63-77
⛔ Skipped due to learnings
Learnt from: landure PR: martin-georgiev/postgresql-for-doctrine#411 File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27 Timestamp: 2025-08-09T15:14:11.841Z Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.docs/INTEGRATING-WITH-SYMFONY.md (1)
35-36: Symfony type registration for ltree looks good.docs/INTEGRATING-WITH-DOCTRINE.md (1)
42-43: Doctrine DBAL type registration for ltree looks good.docs/INTEGRATING-WITH-LARAVEL.md (3)
54-55: Laravel type_mappings: ltree mapping looks correct.
93-94: Custom type mapping for ltree looks correct.
347-349: Runtime registration in DoctrineEventSubscriber is consistent with existing patterns.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
81-82: Fix DBAL 2.x platform detection logic.The current implementation uses
\is_a()with a string class name instead of properinstanceofchecking, and it doesn't properly verify that the class exists before using it.Based on the past review comment, this should be implemented as:
private function assertPostgreSQLPlatform(AbstractPlatform $platform): void { - $isDbalTwoPostgres = \class_exists(PostgreSQLPlatform::class) - && \is_a($platform, '\Doctrine\DBAL\Platforms\PostgreSqlPlatform'); - - if ($platform instanceof PostgreSQLPlatform || $isDbalTwoPostgres) { - return; - } - - throw new \LogicException('Ltree DBAL type can only be used with the PostgreSQL platform.'); + $isPostgres = + $platform instanceof PostgreSQLPlatform + // DBAL 2.x + || ( + \class_exists(\Doctrine\DBAL\Platforms\PostgreSqlPlatform::class) + && $platform instanceof \Doctrine\DBAL\Platforms\PostgreSqlPlatform + ); + + if (!$isPostgres) { + throw new \LogicException('Ltree DBAL type can only be used with the PostgreSQL platform.'); + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(2 hunks)docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
- README.md
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
docs/LTREE-TYPE.md
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
🧬 Code Graph Analysis (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (6)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (3)
InvalidLtreeForDatabaseException(9-25)forInvalidFormat(21-24)forInvalidType(16-19)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (2)
InvalidLtreeForPHPException(9-20)forInvalidType(16-19)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (11)
Ltree(10-136)Override(23-27)Override(29-39)Override(41-45)Override(47-61)Override(63-67)Override(69-73)Override(75-79)Override(81-85)Override(87-91)Override(93-104)src/MartinGeorgiev/Doctrine/DBAL/Types/BaseType.php (2)
BaseType(17-52)getSQLDeclaration(24-29)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
Override(24-25)fromString(19-19)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (1)
Override(21-27)
🔇 Additional comments (3)
docs/LTREE-TYPE.md (1)
1-165: Comprehensive documentation with practical examples.This documentation provides excellent coverage of ltree usage with clear examples for Symfony integration, including entity setup, relationship management, and lifecycle handling. The warning about cascading path updates and the entity listener example are particularly valuable.
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (2)
52-58: Proper exception handling for invalid ltree formats.The implementation correctly catches
InvalidArgumentExceptionfrom the value object'sfromStringmethod and wraps it in a more specific database exception. This provides good error context separation between PHP validation and database format issues.
14-16: Consistent ltree type implementation.The type correctly extends
BaseType, defines the appropriate type name constant, and returns the correct SQL declaration. The implementation follows the established patterns in the codebase.Also applies to: 25-25, 34-34
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
87-96: Critical: DBAL 2.x platform detection is broken (wrong class_exists check).The guard checks class_exists(PostgreSQLPlatform::class) before testing for Doctrine\DBAL\Platforms\PostgreSqlPlatform, which means on DBAL 2.x (where PostgreSQLPlatform doesn’t exist) the 2.x branch is never taken. This will incorrectly throw on valid DBAL 2 PostgreSQL platforms.
Apply this diff to correctly detect both DBAL 2.x and 3.x/4.x:
private function assertPostgreSQLPlatform(AbstractPlatform $platform): void { - $isDbalTwoPostgres = \class_exists(PostgreSQLPlatform::class) - && \is_a($platform, '\Doctrine\DBAL\Platforms\PostgreSqlPlatform'); - - if ($platform instanceof PostgreSQLPlatform || $isDbalTwoPostgres) { - return; - } + $isDbal2Postgres = + \class_exists(\Doctrine\DBAL\Platforms\PostgreSqlPlatform::class) + && $platform instanceof \Doctrine\DBAL\Platforms\PostgreSqlPlatform; + + if ($platform instanceof PostgreSQLPlatform || $isDbal2Postgres) { + return; + } throw new \LogicException('Ltree DBAL type can only be used with the PostgreSQL platform.'); }Alternatively, collapse into a single boolean for readability:
private function assertPostgreSQLPlatform(AbstractPlatform $platform): void { - $isDbalTwoPostgres = \class_exists(PostgreSQLPlatform::class) - && \is_a($platform, '\Doctrine\DBAL\Platforms\PostgreSqlPlatform'); - - if ($platform instanceof PostgreSQLPlatform || $isDbalTwoPostgres) { - return; - } + $isPostgres = + $platform instanceof PostgreSQLPlatform + || (\class_exists(\Doctrine\DBAL\Platforms\PostgreSqlPlatform::class) + && $platform instanceof \Doctrine\DBAL\Platforms\PostgreSqlPlatform); + + if ($isPostgres) { + return; + } throw new \LogicException('Ltree DBAL type can only be used with the PostgreSQL platform.'); }
🧹 Nitpick comments (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
76-85: Nit: short-circuit null earlier for clarity.Returning early on null improves readability slightly and avoids doing unnecessary work.
Apply this diff:
public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string { $this->assertPostgreSQLPlatform($platform); + if (null === $value) { + return null; + } + if (\is_string($value)) { try { $value = LtreeValueObject::fromString($value); } catch (\InvalidArgumentException) { throw InvalidLtreeForPHPException::forInvalidFormat($value); } } if ($value instanceof LtreeInterface) { return (string) $value; } - if (null === $value) { - return null; - } - throw InvalidLtreeForPHPException::forInvalidType($value); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/LTREE-TYPE.md
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
📚 Learning: 2025-08-13T05:48:00.878Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
🧬 Code Graph Analysis (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (5)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (3)
InvalidLtreeForDatabaseException(9-25)forInvalidFormat(21-24)forInvalidType(16-19)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (3)
InvalidLtreeForPHPException(9-25)forInvalidFormat(21-24)forInvalidType(16-19)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (11)
Ltree(10-136)Override(23-27)Override(29-39)Override(41-45)Override(47-61)Override(63-67)Override(69-73)Override(75-79)Override(81-85)Override(87-91)Override(93-104)src/MartinGeorgiev/Doctrine/DBAL/Types/BaseType.php (2)
BaseType(17-52)getSQLDeclaration(24-29)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
Override(24-25)fromString(19-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: sloth
🔇 Additional comments (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (4)
18-26: SQL declaration guarded and correct for PostgreSQL.Returning the literal 'ltree' after asserting PostgreSQL is appropriate here.
29-36: Database type mapping looks good.Exposing 'ltree' via getMappedDatabaseTypes() after the platform guard aligns with DBAL expectations.
52-61: Robust DB-to-PHP conversion with precise error mapping.Parsing strings through LtreeValueObject::fromString() and translating invalid formats to InvalidLtreeForDatabaseException is correct. This also correctly handles empty-string ltrees, which the project accepts as valid (as per retrieved learnings).
68-75: String validation in convertToDatabaseValue resolved the previous gap.Validating raw strings via LtreeValueObject::fromString() avoids letting invalid ltree values reach the DB. Nice improvement.
Contributor
Author
|
Hi @martin-georgiev, what do you think of this PR status ? |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (1)
21-23: Use of #[\Override] in tests is safe across PHP 8.1–8.3, as already discussed.PHP 8.1/8.2 ignore unknown attributes; 8.3 validates overrides. Good call using it here.
🧹 Nitpick comments (4)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (4)
26-27: Avoid executing CREATE EXTENSION on every test — guard it once per process.The IF NOT EXISTS is idempotent, but this still adds a DB round-trip per test. Cache that it ran to reduce overhead.
Apply this diff in setUp():
- $this->createLtreeExtension(); + if (!self::$ltreeExtensionCreated) { + $this->createLtreeExtension(); + self::$ltreeExtensionCreated = true; + }And add this property inside the class (e.g., right after the class declaration):
private static bool $ltreeExtensionCreated = false;
140-151: Broaden provider: add NULL and string-input cases; adjust docblock union type.This will exercise both nullable handling and the “string to ltree” conversion path, complementing the value-object cases. Paired with the assertion tweak below.
Docblock and cases update:
- /** - * @return array<string, array{string, string, ?LtreeValueObject}> - */ + /** + * @return array<string, array{string, string, string|LtreeValueObject|null}> + */ public static function provideLtreeTypeTestCases(): array { return [ 'ltree simple string' => ['ltree', 'LTREE', new LtreeValueObject(['foo', 'bar', 'baz'])], + 'ltree from string' => ['ltree', 'LTREE', 'foo.bar.baz'], 'ltree simple numeric' => ['ltree', 'LTREE', new LtreeValueObject(['1', '2', '3'])], 'ltree single numeric' => ['ltree', 'LTREE', new LtreeValueObject(['1'])], 'ltree empty' => ['ltree', 'LTREE', new LtreeValueObject([])], + 'ltree null' => ['ltree', 'LTREE', null], ]; }
221-229: Handle ltree expected strings in the generic equality helper.When inserting a string (provider case), convert the actual Ltree back to string for comparison.
private function assertDatabaseRoundtripEquals(mixed $expected, mixed $actual, string $typeName): void { match (true) { $expected instanceof PointValueObject => $this->assertPointEquals($expected, $actual, $typeName), $expected instanceof RangeValueObject => $this->assertRangeEquals($expected, $actual, $typeName), $expected instanceof LtreeValueObject => $this->assertLtreeEquals($expected, $actual, $typeName), + $typeName === 'ltree' && \is_string($expected) => $this->assertSame($expected, (string) $actual, 'Failed asserting that ltree string representations are identical for type '.$typeName), \is_array($expected) => $this->assertEquals($expected, $actual, 'Failed asserting that array values are equal for type '.$typeName), default => $this->assertSame($expected, $actual, 'Failed asserting that values are identical for type '.$typeName) }; }
249-253: Improve diagnostics by comparing string representations.Comparing (string) forms gives clearer diffs than a boolean equals() check when failures happen.
private function assertLtreeEquals(LtreeValueObject $ltreeValueObject, mixed $actual, string $typeName): void { $this->assertInstanceOf(LtreeValueObject::class, $actual, 'Failed asserting that value is a Ltree object for type '.$typeName); - $this->assertTrue($ltreeValueObject->equals($actual), 'Failed asserting that values are equal for type '.$typeName); + $this->assertSame((string) $ltreeValueObject, (string) $actual, 'Failed asserting that ltree string representations are identical for type '.$typeName); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.md(2 hunks)docs/AVAILABLE-TYPES.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(2 hunks)docs/INTEGRATING-WITH-LARAVEL.md(5 hunks)docs/INTEGRATING-WITH-SYMFONY.md(5 hunks)docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php(5 hunks)tests/Integration/MartinGeorgiev/TestCase.php(2 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- docs/AVAILABLE-TYPES.md
- tests/Integration/MartinGeorgiev/TestCase.php
- README.md
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
- docs/INTEGRATING-WITH-SYMFONY.md
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
- docs/INTEGRATING-WITH-DOCTRINE.md
- docs/INTEGRATING-WITH-LARAVEL.md
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
- docs/LTREE-TYPE.md
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-08-13T05:48:00.878Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-03-26T02:46:12.804Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-26T02:46:12.804Z
Learning: The PR "preserve the type of floats and integers when transforming back and forth between PostgreSQL and PHP" improves type handling by ensuring that integers remain integers, floats remain floats, numeric strings stay as strings, and booleans are properly converted through the transformation process.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-04-20T11:24:18.300Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-08-09T15:14:11.841Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27
Timestamp: 2025-08-09T15:14:11.841Z
Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php
🧬 Code Graph Analysis (1)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
Ltree(14-97)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
Ltree(10-136)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
equals(39-39)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php (5)
setUp(25-30)DataProvider(61-66)DataProvider(68-73)DataProvider(100-106)DataProvider(122-128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
🔇 Additional comments (3)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (3)
11-11: Import alias for Ltree value object is correct and consistent.Using an alias keeps the test readable and aligned with existing patterns in this file.
134-139: Generic runTypeTest reuse for ltree is clean.Leveraging the existing harness keeps the new type consistent with the other DBAL types’ integration tests.
255-259: Creating the ltree extension in the test harness is appropriate.Using IF NOT EXISTS prevents flakiness when the extension is already present.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
docs/LTREE-TYPE.md (2)
12-18: Symfony migration phrasing nitMinor clarity/grammar tweak in the Symfony instructions.
-For [Symfony](https://symfony.com/), -customize the migration introducing the `ltree` field by adding this line -at the beginning of the `up()` method: +For [Symfony](https://symfony.com/), +customize the migration that introduces the `ltree` field by adding this line +at the beginning of the `up()` method:
138-146: Tighten wording of the cascading noteImprove clarity and brevity.
-⚠️ **Important**: Changing an entity's parent requires to cascade the change -to all its children. -This is not handled automatically by Doctrine. -Implement a [preUpdate](https://www.doctrine-project.org/projects/doctrine-orm/en/3.3/reference/events.html#reference-events-pre-update) -[Doctrine Entity Listener](https://www.doctrine-project.org/projects/doctrine-orm/en/3.3/reference/events.html#entity-listeners-class) -([Doctrine Entity Listener @ Symfony](https://symfony.com/doc/current/doctrine/events.html#doctrine-entity-listeners)) -to handle updating the `path` column of the updated entity children -when the `path` is present in the changed fields: +⚠️ **Important**: Changing an entity’s parent requires cascading the change +to all of its children. +Doctrine does not handle this automatically. +Implement a [preUpdate](https://www.doctrine-project.org/projects/doctrine-orm/en/3.3/reference/events.html#reference-events-pre-update) +[Doctrine Entity Listener](https://www.doctrine-project.org/projects/doctrine-orm/en/3.3/reference/events.html#entity-listeners-class) +([Symfony docs](https://symfony.com/doc/current/doctrine/events.html#doctrine-entity-listeners)) +to update the `path` of the affected entity’s children when `path` changes:src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
47-51: Fix typo and clarify exception conditionThere’s a typo in the tag, and “root” should be unambiguous (i.e., empty path).
/** - * @tthrows \LogicException if the ltree is root + * @throws \LogicException if the ltree is root (empty path) */ public function getParent(): static;src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (2)
119-123: Prefer foreach over array_map for validation side effectsarray_map is used for side effects, which is non-idiomatic and may create an unnecessary array. A simple foreach improves clarity and avoids allocations.
- \array_map( - self::assertNonEmptyString(...), - $value, - ); + foreach ($value as $v) { + self::assertNonEmptyString($v); + }
47-56: Optional: validate labels against PostgreSQL ltree rulesBeyond dot checks, ltree labels have stricter character rules (letters, digits, underscore; no dots; length limits). You may want to enforce this in PHP for earlier feedback instead of deferring to the database.
Example:
public function withLeaf(string $leaf): static { if ('' === $leaf) { throw new \InvalidArgumentException('Leaf cannot be empty.'); } if (\str_contains($leaf, '.')) { throw new \InvalidArgumentException('Leaf cannot contain dot.'); } + + // Optional: align with ltree's ltxtquery/label rules (simplified) + if (!\preg_match('/^[A-Za-z0-9_]+$/', $leaf)) { + throw new \InvalidArgumentException('Leaf must contain only letters, digits, or underscores.'); + }If you prefer to rely solely on DB-side validation, feel free to skip this.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
📚 Learning: 2025-08-13T05:48:00.878Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: The PostgreSQL-for-doctrine library implements the JSON array element existence check using the `RIGHT_EXISTS_ON_LEFT` DQL function, which maps to PostgreSQL's `?` operator. This is used for checking if a specific value exists in a JSONB array column.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: When using operator-like functions in PostgreSQL-for-doctrine, they must be used as boolean functions with `= TRUE` rather than as direct operators (e.g., `RIGHT_EXISTS_ON_LEFT(column, value) = TRUE` instead of `column RIGHT_EXISTS_ON_LEFT value`).
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
🧬 Code Graph Analysis (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (11)
__construct(17-21)Override(23-27)Override(29-39)Override(41-45)Override(47-61)Override(63-67)Override(69-73)Override(75-79)Override(81-85)Override(87-91)Override(93-104)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (3)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (5)
Ltree(14-97)Override(18-26)Override(28-36)Override(43-61)Override(63-85)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (10)
__construct(14-14)Override(24-25)fromString(19-19)withLeaf(32-32)getPathFromRoot(37-37)equals(39-39)isAncestorOf(41-41)isLeafOf(43-43)isRoot(45-45)getParent(50-50)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php (1)
Override(21-27)
🪛 LanguageTool
docs/LTREE-TYPE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ....postgresql.org/docs/current/ltree.html) in PostgreSQL. ```sql CREATE EXTENSION ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...`` For Symfony, customize the migration introducing the ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ng the ltree field by adding this line at the beginning of the up() method: ...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...'s parent requires to cascade the change to all its children. This is not handled...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ... cascade the change to all its children. This is not handled automatically by Doc...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...events.html#reference-events-pre-update) [Doctrine Entity Listener](https://www.d...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...ence/events.html#entity-listeners-class) ([Doctrine Entity Listener @ Symfony](ht...
(QB_NEW_EN)
[grammar] ~143-~143: There might be a mistake here.
Context: .../events.html#doctrine-entity-listeners)) to handle updating the path column of ...
(QB_NEW_EN)
[grammar] ~144-~144: There might be a mistake here.
Context: ...hcolumn of the updated entity children when thepath` is present in the change...
(QB_NEW_EN)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
Outdated
Show resolved
Hide resolved
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
Outdated
Show resolved
Hide resolved
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
9-15: Constructor PHPDoc: clarify empty path validity and align throws with array contract (not string-specific).
- The constructor accepts a list of labels (array form), where an empty list represents the empty path/root. Please make this explicit.
- The current throws description mentions “consecutive dots” and leading/trailing dots, which apply to the string form, not the array form. Tighten the description to match the actual validation performed in Ltree::__construct/assertListOfValidLtreeNodes.
Apply:
- /** - * @param list<non-empty-string> $pathFromRoot - * - * @throws \InvalidArgumentException if the pathFromRoot is not a valid ltree path - * (contains empty labels, consecutive dots, or starts/ends with a dot) - */ + /** + * @param list<non-empty-string> $pathFromRoot Non-empty labels; the list itself may be empty to represent the root. + * @throws \InvalidArgumentException if the list is not a list of non-empty labels (labels must not contain '.') + */ public function __construct(array $pathFromRoot);
17-22: fromString() PHPDoc: fix grammar and state empty-string is valid root.
- Grammar: “is contains” → “contains”.
- Explicitly note that '' is allowed and represents the empty path (consistent with implementation and tests).
/** - * Creates an Ltree instance from a string representation. - * - * @throws \InvalidArgumentException if the ltree is contains invalid/empty labels (consecutive dots) + * Creates an Ltree instance from a string representation. An empty string ('') represents the empty path. + * + * @throws \InvalidArgumentException if the ltree contains invalid/empty labels (e.g., consecutive dots) or starts/ends with a dot */ public static function fromString(string $ltree): static;
🧹 Nitpick comments (11)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
62-69: Minor language tweak in withLeaf() PHPDoc.Improve clarity/grammar: “contains a dot”.
/** * Creates a new Ltree instance with the given leaf added to the end of the path. * * @param non-empty-string $leaf * - * @throws \InvalidArgumentException if the leaf is empty or contains dot + * @throws \InvalidArgumentException if the leaf is empty or contains a dot */ public function withLeaf(string $leaf): static;docs/LTREE-TYPE.md (4)
12-18: Symfony migration phrasing: minor grammar tightening.Improve the sentence flow.
-For [Symfony](https://symfony.com/), -customize the migration introducing the `ltree` field by adding this line -at the beginning of the `up()` method: +For [Symfony](https://symfony.com/), customize the migration that introduces the `ltree` field by adding this line +at the beginning of the `up()` method:
36-36: Remove unused import in example.Doctrine DBAL Types alias is not used in the snippet; drop it to avoid confusion.
-use Doctrine\DBAL\Types\Types;
141-145: Use SQL comment style inside SQL block.Use “--” instead of “//” in SQL examples.
-// Example GiST index for ltree with a custom signature length (must be a multiple of 4) +-- Example GiST index for ltree with a custom signature length (must be a multiple of 4) CREATE INDEX my_entity_path_gist_idx ON my_entity USING GIST (path gist_ltree_ops(siglen = 100));
147-154: Grammar: “requires to cascade” → “requires cascading”, and minor clarity.Tighten phrasing and readability.
-⚠️ **Important**: Changing an entity's parent requires to cascade the change -to all its children. -This is not handled automatically by Doctrine. +⚠️ **Important**: Changing an entity's parent requires cascading the change +to all its children. +This is not handled automatically by Doctrine.src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (4)
36-39: Drop phpstan-ignore by asserting before constructing.Refine static analysis and keep validation centralized by calling the assert method prior to instantiation.
- $pathFromRoot = \explode('.', $ltree); - - return new static($pathFromRoot); // @phpstan-ignore-line argument.type + $pathFromRoot = \explode('.', $ltree); + self::assertListOfValidLtreeNodes($pathFromRoot); + /** @var list<non-empty-string> $pathFromRoot */ + return new static($pathFromRoot);
95-96: Avoid sprintf for simple concatenation in prefix checks.Slight readability/perf improvement; also reduces temporary allocations.
- return \str_starts_with((string) $ltree, \sprintf('%s.', (string) $this)); + $prefix = (string) $this . '.'; + return \str_starts_with((string) $ltree, $prefix);
109-110: Mirror the prefix optimization in isDescendantOf().Consistency with isAncestorOf() change.
- return \str_starts_with((string) $this, \sprintf('%s.', (string) $ltree)); + $prefix = (string) $ltree . '.'; + return \str_starts_with((string) $this, $prefix);
159-169: Replace array_map with foreach to avoid unnecessary array allocations.array_map’s return is unused; foreach is clearer and avoids building an intermediate array.
- \array_map( - self::assertValidLtreeNode(...), - $value, - ); + foreach ($value as $node) { + self::assertValidLtreeNode($node); + }tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (2)
96-101: Remove no-op call in test_get_parent().$child->getPathFromRoot() has no effect on the test outcome.
- $child->getPathFromRoot(); $ltree = $child->getParent();
20-40: Consider adding negative tests for fromString() invalid formats.You already cover invalid constructor arrays. Adding explicit invalid string cases strengthens coverage for the string parser edge cases (e.g., consecutive dots, leading/trailing dots).
Example addition (outside this hunk):
public function test_from_string_throws_on_invalid_formats(): void { $this->expectException(\InvalidArgumentException::class); Ltree::fromString('a..b'); } public function test_from_string_throws_on_leading_dot(): void { $this->expectException(\InvalidArgumentException::class); Ltree::fromString('.a'); } public function test_from_string_throws_on_trailing_dot(): void { $this->expectException(\InvalidArgumentException::class); Ltree::fromString('a.'); }If you want, I can prepare a PR-ready patch adding these tests with a small data provider.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
📚 Learning: 2025-08-13T05:48:00.878Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-14T18:59:27.438Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.438Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override parent interface methods, including when adding more specific type annotations. In LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types, #[\Override] is the correct usage and provides compile-time validation.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-14T18:59:27.438Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.438Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override or redefine methods from parent interfaces, including when adding more specific type annotations. This applies to cases like LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-09T15:14:11.841Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27
Timestamp: 2025-08-09T15:14:11.841Z
Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: The PostgreSQL-for-doctrine library implements the JSON array element existence check using the `RIGHT_EXISTS_ON_LEFT` DQL function, which maps to PostgreSQL's `?` operator. This is used for checking if a specific value exists in a JSONB array column.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: When using operator-like functions in PostgreSQL-for-doctrine, they must be used as boolean functions with `= TRUE` rather than as direct operators (e.g., `RIGHT_EXISTS_ON_LEFT(column, value) = TRUE` instead of `column RIGHT_EXISTS_ON_LEFT value`).
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
🧬 Code Graph Analysis (3)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (15)
__construct(17-21)Override(23-27)Override(29-39)Override(41-45)Override(47-51)Override(53-64)Override(66-70)Override(72-76)Override(78-82)Override(84-96)Override(98-110)Override(112-120)Override(122-130)Override(132-140)Override(142-150)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (2)
Override(18-26)Override(28-36)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (5)
Ltree(14-97)Override(18-26)Override(28-36)Override(43-61)Override(63-85)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (13)
__construct(15-15)Override(27-28)fromString(22-22)getPathFromRoot(33-33)getParent(38-38)isEmpty(45-45)equals(40-40)isRoot(50-50)isAncestorOf(52-52)isDescendantOf(54-54)isParentOf(56-56)isChildOf(58-58)withLeaf(69-69)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
Ltree(10-186)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (6)
fromString(22-22)getPathFromRoot(33-33)getParent(38-38)isEmpty(45-45)isRoot(50-50)withLeaf(69-69)
🪛 LanguageTool
docs/LTREE-TYPE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ....postgresql.org/docs/current/ltree.html) in PostgreSQL. ```sql CREATE EXTENSION ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...`` For Symfony, customize the migration introducing the ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ng the ltree field by adding this line at the beginning of the up() method: ...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...postgresql.org/docs/current/gist.html). Add a GiST index to an ltree column by ...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...mn by manually adding its CREATE INDEX command to the migration: ```sql // Exa...
(QB_NEW_EN)
[grammar] ~147-~147: There might be a mistake here.
Context: ...'s parent requires to cascade the change to all its children. This is not handled...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ... cascade the change to all its children. This is not handled automatically by Doc...
(QB_NEW_EN)
[grammar] ~150-~150: There might be a mistake here.
Context: ...events.html#reference-events-pre-update) [Doctrine Entity Listener](https://www.d...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...ence/events.html#entity-listeners-class) ([Doctrine Entity Listener @ Symfony](ht...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: .../events.html#doctrine-entity-listeners)) to handle updating the path column of ...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ...hcolumn of the updated entity children when thepath` is present in the change...
(QB_NEW_EN)
🔇 Additional comments (3)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
27-28: #[\Override] on interface method is correct and forward-compatible.Good use of #[\Override] for a method redeclared from JsonSerializable with a stricter return type. Per our prior learnings, this is valid in PHP 8.3+ and ignored in 8.1/8.2.
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
78-83: isRoot() semantics match interface and tests.Only a single-label path is considered root; empty path is not. This aligns with getParent() and the documented semantics.
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (1)
144-152: Relationship semantics test for isRoot() are accurate.Empty path is not root; single label is root; multi-label is not. Matches implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
docs/LTREE-TYPE.md (5)
5-6: Use PostgreSQL terminology: say “extension”, not “module”.Minor wording fix to align with PostgreSQL docs.
-The `ltree` data type requires enabling the [`ltree` module](https://www.postgresql.org/docs/current/ltree.html) +The `ltree` data type requires enabling the [`ltree` extension](https://www.postgresql.org/docs/current/ltree.html) in PostgreSQL.
16-18: Consider omittingWITH SCHEMA publicto reduce required privileges (optional).Some managed environments restrict
WITH SCHEMA. Defaulting to the current schema is often fine and simpler.-$this->addSql('CREATE EXTENSION IF NOT EXISTS ltree WITH SCHEMA public'); +$this->addSql('CREATE EXTENSION IF NOT EXISTS ltree');If your deployment requires the objects in a specific schema, keep the explicit
WITH SCHEMA ....
117-132: Prefer value-object helpers for cycle detection.Use the
Ltreerelationship predicates for clarity and correctness rather than raw array checks.- if (in_array($this->getId()->toBase58(), $parent->getPath()->getPathFromRoot(), true)) { - throw new \InvalidArgumentException("Parent MyEntity can't be a child of the current MyEntity"); - } + // Prevent cycles: the parent can't be a descendant of the current node. + if ($parent->getPath()->isDescendantOf($this->getPath())) { + throw new \InvalidArgumentException("Parent MyEntity can't be a child of the current MyEntity"); + }
136-144: Also document GIN index as an alternative (optional).GiST is great; many use GIN for ltree as well. Adding a GIN example helps readers choose per workload.
Additional example:
-- Alternative: GIN index for ltree CREATE INDEX my_entity_path_gin_idx ON my_entity USING GIN (path gin_ltree_ops);
146-153: Clarify cascade scope (children vs descendants) and consider onFlush for deep trees.The snippet updates direct children only. If you intend to propagate to all descendants, you’ll need recursion (or an iterative traversal) and, ideally, an
onFlushlistener to safely schedule updates.Would you like an example
onFlushlistener that:
- walks descendants,
- updates paths via
setParent,- and recomputes/schedules change sets for all affected nodes?
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
10-14: Document that an empty list represents the root (nit).This matches the implementation and PostgreSQL behavior; stating it explicitly helps consumers.
- * @param list<non-empty-string> $pathFromRoot + * @param list<non-empty-string> $pathFromRoot The list may be empty to represent the root.
20-21: Fix grammar and broaden the error description.Minor grammar fix and clarify that invalid characters can also trigger an exception.
- * @throws \InvalidArgumentException if the ltree is contains invalid/empty labels (consecutive dots) + * @throws \InvalidArgumentException if the ltree contains invalid/empty labels (e.g., consecutive dots) or illegal characters
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
📚 Learning: 2025-08-13T05:48:00.878Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.878Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-14T18:59:27.438Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.438Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override parent interface methods, including when adding more specific type annotations. In LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types, #[\Override] is the correct usage and provides compile-time validation.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-14T18:59:27.438Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.438Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override or redefine methods from parent interfaces, including when adding more specific type annotations. This applies to cases like LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-09T15:14:11.841Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27
Timestamp: 2025-08-09T15:14:11.841Z
Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
🧬 Code Graph Analysis (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (15)
__construct(17-21)Override(23-27)Override(29-41)Override(43-47)Override(49-53)Override(55-65)Override(67-71)Override(73-77)Override(79-83)Override(85-99)Override(101-115)Override(117-125)Override(127-135)Override(137-145)Override(147-155)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (2)
Override(18-26)Override(28-36)
🪛 LanguageTool
docs/LTREE-TYPE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ....postgresql.org/docs/current/ltree.html) in PostgreSQL. ```sql CREATE EXTENSION ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...`` For Symfony, customize the migration that introduces ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...es the ltree field by adding this line at the beginning of the up() method: ...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...postgresql.org/docs/current/gist.html). Add a GiST index to an ltree column by ...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...mn by manually adding its CREATE INDEX command to the migration: ```sql -- Exa...
(QB_NEW_EN)
[grammar] ~149-~149: There might be a mistake here.
Context: ...events.html#reference-events-pre-update) [Doctrine Entity Listener](https://www.d...
(QB_NEW_EN)
[grammar] ~150-~150: There might be a mistake here.
Context: ...ence/events.html#entity-listeners-class) ([Doctrine Entity Listener @ Symfony](ht...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: .../events.html#doctrine-entity-listeners)) to handle updating the path column of ...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...hcolumn of the updated entity children when thepath` is present in the change...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
🔇 Additional comments (4)
docs/LTREE-TYPE.md (3)
49-51: LGTM: DB column mapping to customltreetype looks correct.The property typing to
LtreeInterfaceand the Doctrine column typeltreealign with the DBAL Type introduced in this PR.
71-76: LGTM: Safe path initialization before optionally setting the parent.Initializing the path with the entity’s ID and then calling
setParent()is consistent with the value object constraints and avoids circularities early.
165-183: LGTM: Recomputing child change sets ensures persistence.Recomputing change sets in
preUpdateis the minimal fix to have Doctrine pick up child mutations.src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (1)
27-28: LGTM:#[\Override]on interface method works for PHP 8.3+ and is forward-compatible.Keeps intent clear and is ignored by PHP 8.1/8.2.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
10-14: Clarify constructor PHPDoc: distinguish empty path vs. “root/top-level” nodeThe current wording mixes “root” with “empty.” Suggest tightening phrasing and simplifying the invalid-label clause.
- * @param list<non-empty-string> $pathFromRoot A list with one element represents the root. The list may be empty. - * - * @throws \InvalidArgumentException if the pathFromRoot is not a valid ltree path - * (contains labels which are empty or contains one or more dots) + * @param list<non-empty-string> $pathFromRoot List of labels; may be empty to represent an empty path. + * Exactly one element denotes a top-level node. + * + * @throws \InvalidArgumentException if $pathFromRoot is not a valid ltree path + * (e.g., empty labels or labels containing dots)
48-50: Optional: align terminology with constructor docIf adopting “top-level node” above, consider mirroring it here to avoid reader confusion across the interface.
- * Checks if the ltree has only one node. + * Checks if the ltree has exactly one label (i.e., a top‑level node).docs/LTREE-TYPE.md (3)
39-45: Index annotation yields a default B‑tree; prefer migration‑only GiST/GINThe attribute creates a B-tree index by default, which is unsuitable for ltree. While your note about manually editing the migration works, it’s easy to forget and can cause drift. Consider omitting the attribute and managing the GiST/GIN index solely via migrations.
#[ORM\Entity()] -#[ORM\Index(columns: ['path'], name: 'my_entity_path_gist_idx')] class MyEntity implements \StringableIf you keep the attribute to retain the index name, add a bold warning right next to it that the generated SQL must be edited to “USING GIST (path gist_ltree_ops(...))” (or GIN), and that schema diffs should be migration-only (not schema-tool:update).
154-160: Grammar/clarity nits in the event-listener sectionMinor wording tweaks improve readability.
-Implement a [onFlush](https://www.doctrine-project.org/projects/doctrine-orm/en/3.3/reference/events.html#reference-events-on-flush) +Implement an [onFlush](https://www.doctrine-project.org/projects/doctrine-orm/en/3.3/reference/events.html#reference-events-on-flush) [Doctrine Entity Listener](https://symfony.com/doc/current/doctrine/events.html#doctrine-lifecycle-listeners) -to handle updating the `path` column of the updated entity children -when the `path` is present in the changed fields: +to update the `path` column of the updated entity’s children +when `path` is among the changed fields:
140-152: Add one more tip: siglen must be a multiple of 4 — already noted; consider linking to official docs for opclassesThe examples are solid. As an optional enhancement, add a short note that different workloads may prefer GIN over GiST and include a link to the PostgreSQL ltree indexing docs so readers can choose the opclass appropriately.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
📚 Learning: 2025-08-13T05:48:00.893Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.893Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-14T18:59:27.478Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.478Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override parent interface methods, including when adding more specific type annotations. In LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types, #[\Override] is the correct usage and provides compile-time validation.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-14T18:59:27.478Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.478Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override or redefine methods from parent interfaces, including when adding more specific type annotations. This applies to cases like LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-09T15:14:11.841Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27
Timestamp: 2025-08-09T15:14:11.841Z
Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
🧬 Code graph analysis (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (15)
__construct(17-21)Override(23-27)Override(29-41)Override(43-47)Override(49-53)Override(55-65)Override(67-71)Override(73-77)Override(79-83)Override(85-99)Override(101-115)Override(117-125)Override(127-135)Override(137-145)Override(147-155)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (2)
Override(18-26)Override(28-36)
🪛 LanguageTool
docs/LTREE-TYPE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ....postgresql.org/docs/current/ltree.html) in PostgreSQL. ```sql CREATE EXTENSION ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...`` For Symfony, customize the migration that introduces ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...es the ltree field by adding this line at the beginning of the up() method: ...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ....postgresql.org/docs/current/gist.html) or [GIN indexes](https://www.postgresql.o...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ....postgresql.org/docs/current/gin.html). Add a GiST index to an ltree column by ...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...mn by manually adding its CREATE INDEX command to the migration: ```sql -- Exa...
(QB_NEW_EN)
[grammar] ~157-~157: There might be a mistake here.
Context: ...e/events.html#reference-events-on-flush) [Doctrine Entity Listener](https://symfo...
(QB_NEW_EN)
[grammar] ~158-~158: There might be a mistake here.
Context: ...vents.html#doctrine-lifecycle-listeners) to handle updating the path column of ...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ...hcolumn of the updated entity children when thepath` is present in the change...
(QB_NEW_EN)
🔇 Additional comments (3)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
27-28: Keep #[\Override] on interface jsonSerialize() — correct and future-proofUsing #[\Override] here is valid (PHP 8.3+) and helpful for intent and compile-time validation; it’s ignored on 8.1/8.2. No action needed.
20-22: fromString() doc now matches behavior (empty string accepted) — looks goodAccurately documents rejection of invalid/empty labels (e.g., ".."). Alignment with implementation is solid.
docs/LTREE-TYPE.md (1)
81-86: LGTM: #[\Override] on __toString()Good use to assert interface conformance and intent. No change needed.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
10-15: Clarify “root” vs “empty path” semantics in PHPDocRight now it says “A list with one element represents the root. The list may be empty.” To remove any ambiguity, explicitly distinguish between:
- empty path ([]) = no labels
- root-level node = exactly one label
Apply:
- * @param list<non-empty-string> $pathFromRoot A list with one element represents the root. The list may be empty. + * @param list<non-empty-string> $pathFromRoot Path from the root: + * - [] means “no labels” (empty path) + * - [X] means a single root-level node
17-22: Document empty-string handling for fromString()The implementation accepts '' and returns an empty path. Make that explicit in the PHPDoc to align expectations.
/** * Creates an Ltree instance from a string representation. * - * @throws \InvalidArgumentException if $ltree contains invalid/empty labels (e.g., consecutive dots) + * An empty string ('') yields an empty path ([]). + * + * @throws \InvalidArgumentException if $ltree contains invalid/empty labels (e.g., consecutive dots) */README.md (1)
61-62: Nice addition of Hierarchical TypesListing ltree under “Data Types” with a link to upstream docs improves discoverability. Consider adding a short note “requires enabling the ltree extension” to set expectations.
- - [ltree](https://www.postgresql.org/docs/current/ltree.html) (`ltree`) + - [ltree](https://www.postgresql.org/docs/current/ltree.html) (`ltree`) — requires enabling the PostgreSQL ltree extensiondocs/LTREE-TYPE.md (4)
12-18: Migration tip is practicalAdding CREATE EXTENSION in the migration’s up() is the right call. Consider adding the complementary DROP EXTENSION in down() for symmetry (teams often appreciate reversible migrations).
$this->addSql('CREATE EXTENSION IF NOT EXISTS ltree'); +// optionally in down(): +// $this->addSql('DROP EXTENSION IF EXISTS ltree');
43-45: Index declaration: consider avoiding ORM Index attribute for ltreeDeclaring #[ORM\Index] produces a B-tree definition at the ORM level, while the migration is edited to GiST/GIN. This mismatch can cause confusing diffs later. An alternative is to omit the attribute and manage the GiST/GIN index solely via migrations.
-#[ORM\Index(columns: ['path'], name: 'my_entity_path_gist_idx')] +// Consider dropping the ORM Index attribute and keeping the GiST/GIN index solely in migrationsIf you prefer to keep the attribute for “don’t drop” semantics, please confirm your team is fine with schema diffs not reflecting the access method.
140-152: Clear GiST/GIN index examplesGood guidance, including siglen note for GiST. You may add a quick “choose GiST for containment queries with operators @>, <@; GIN can be faster for existence/search patterns” footnote, but optional.
176-225: onFlush listener correctly cascades path updates
- Uses scheduled updates set to detect path changes
- Recomputes each child change set after mutation
- Recurses through descendants
Looks good. For very deep trees consider tail-recursive or iterative traversal to avoid deep recursion limits, but that’s an edge case.
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php (3)
52-59: Tighten the guard condition for readabilityAdd parentheses to make intent explicit and keep the exception message accurate.
- if (!$expected instanceof LtreeValueObject && !\is_string($expected) || !$actual instanceof LtreeValueObject) { + if ((!$expected instanceof LtreeValueObject && !\is_string($expected)) || !$actual instanceof LtreeValueObject) { throw new \InvalidArgumentException('LtreeTypeTest expects Ltree value objects.'); }Alternatively, consider early returns:
+ if (!$actual instanceof LtreeValueObject) { + $this->fail('Actual value must be a Ltree object for type '.$typeName); + } + if (!$expected instanceof LtreeValueObject && !\is_string($expected)) { + $this->fail('Expected must be a Ltree object or string for type '.$typeName); + }
63-69: Data provider key parameter is unusedPHPMD flags $testName as unused. Either use it in the assertion message or prefix with an underscore.
- public function can_handle_ltree_values(string $testName, LtreeValueObject $ltreeValueObject): void + public function can_handle_ltree_values(string $_testName, LtreeValueObject $ltreeValueObject): voidOr include it in failure messages for better diagnostics.
71-82: Adjust docblock to match provided valuesProvider never yields null for the value object, so drop the nullable.
- * @return array<string, array{string, ?LtreeValueObject}> + * @return array<string, array{string, LtreeValueObject}>Also consider adding a case that verifies empty-string roundtrip ('') via the string path (distinct from the VO case).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
README.md(2 hunks)docs/AVAILABLE-TYPES.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(2 hunks)docs/INTEGRATING-WITH-LARAVEL.md(5 hunks)docs/INTEGRATING-WITH-SYMFONY.md(5 hunks)docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php(1 hunks)tests/Integration/MartinGeorgiev/TestCase.php(2 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/AVAILABLE-TYPES.md
🚧 Files skipped from review as they are similar to previous changes (9)
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
- tests/Integration/MartinGeorgiev/TestCase.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
- docs/INTEGRATING-WITH-DOCTRINE.md
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
- docs/INTEGRATING-WITH-SYMFONY.md
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.phpdocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.phpdocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.phpdocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-08-13T05:48:00.893Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.893Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-14T18:59:27.478Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.478Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override parent interface methods, including when adding more specific type annotations. In LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types, #[\Override] is the correct usage and provides compile-time validation.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-14T18:59:27.478Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:24-26
Timestamp: 2025-08-14T18:59:27.478Z
Learning: The #[\Override] attribute in PHP 8.3+ is valid and appropriate for interface methods that override or redefine methods from parent interfaces, including when adding more specific type annotations. This applies to cases like LtreeInterface extending \JsonSerializable and redefining jsonSerialize() with stricter return types.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-09T15:14:11.841Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27
Timestamp: 2025-08-09T15:14:11.841Z
Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php
📚 Learning: 2025-08-19T13:07:15.170Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.170Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what TYPE_NAME constants contain and getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-08-19T13:07:15.170Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.170Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-08-19T13:07:15.170Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.170Z
Learning: In martin-georgiev/postgresql-for-doctrine, the point[] array type should be documented as "point[]" not "_point" in the AVAILABLE-TYPES.md table, to be consistent with all other array types like text[], jsonb[], inet[], etc.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.mdREADME.md
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
🧬 Code graph analysis (2)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
Ltree(14-97)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
Ltree(10-190)tests/Integration/MartinGeorgiev/TestCase.php (2)
TestCase(45-265)setUp(64-93)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/TestCase.php (1)
runTypeTest(31-73)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (13)
__construct(17-21)Override(23-27)Override(29-41)Override(43-47)Override(49-53)Override(55-65)Override(67-71)Override(73-77)Override(79-83)Override(85-99)Override(101-115)Override(117-125)Override(127-135)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (4)
Override(18-26)Override(28-36)Override(43-61)Override(63-85)
🪛 PHPMD (2.15.0)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php
63-63: Avoid unused parameters such as '$testName'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 LanguageTool
README.md
[grammar] ~61-~61: There might be a mistake here.
Context: ...ge, int8range`) - Hierarchical Types - [ltree](https://www.postgresql.org/docs/c...
(QB_NEW_EN)
docs/LTREE-TYPE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ....postgresql.org/docs/current/ltree.html) in PostgreSQL. ```sql CREATE EXTENSION ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...`` For Symfony, customize the migration that introduces ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...es the ltree field by adding this line at the beginning of the up() method: ...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ....postgresql.org/docs/current/gist.html) or [GIN indexes](https://www.postgresql.o...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ....postgresql.org/docs/current/gin.html). Add a GiST index to an ltree column by ...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...mn by manually adding its CREATE INDEX command to the migration: ```sql -- Exa...
(QB_NEW_EN)
[grammar] ~157-~157: There might be a mistake here.
Context: ...e/events.html#reference-events-on-flush) [Doctrine Entity Listener](https://symfo...
(QB_NEW_EN)
[grammar] ~158-~158: There might be a mistake here.
Context: ...vents.html#doctrine-lifecycle-listeners) to handle updating the path column of ...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ...hcolumn of the updated entity children when thepath` is present in the change...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: sloth
🔇 Additional comments (9)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php (2)
27-28: Use of #[\Override] on interface method is correctAnnotating jsonSerialize() here is valid for PHP 8.3+ and safely ignored on 8.1/8.2. Good use for intent and compile-time checks.
36-38: Consistent exception contract for getParent()The interface documents a LogicException for empty ltress; the implementation throws exactly that. All good.
README.md (1)
119-120: Helpful deep link to usage guideLinking to docs/LTREE-TYPE.md from README is spot on.
docs/INTEGRATING-WITH-LARAVEL.md (3)
54-55: Add type mapping for ltree (Laravel config)The new 'ltree' => 'ltree' mapping looks correct and consistent with other scalar types.
93-94: Register custom type class for ltreeCorrectly wiring the DBAL type to MartinGeorgiev\Doctrine\DBAL\Types\Ltree::class. Good.
347-349: EventSubscriber: runtime registration for ltreeSolid addition. Matches the pattern for other types and prevents “Unknown type” issues on fresh connections.
docs/LTREE-TYPE.md (1)
120-136: Cycle prevention and path recomputation look correct
- Self-parent guard ✅
- Descendant-as-parent guard via isDescendantOf ✅
- Path recomputation via withLeaf() ✅
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php (2)
90-94: Good: create extension in setUp to ensure environmentEnsures tests don’t fail on clean databases. Nice.
46-47: Invalid ltree label in string test ('grand-child' contains a hyphen)PostgreSQL ltree labels do not allow hyphens. This will fail during conversion or at insert time. Use an underscore or alphanumerics.
- $this->runTypeTest($typeName, $columnType, 'root.child.grand-child'); + $this->runTypeTest($typeName, $columnType, 'root.child.grand_child');⛔ Skipped due to learnings
Learnt from: landure PR: martin-georgiev/postgresql-for-doctrine#411 File: docs/LTREE-TYPE.md:72-77 Timestamp: 2025-08-13T05:48:00.893Z Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.Learnt from: landure PR: martin-georgiev/postgresql-for-doctrine#411 File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66 Timestamp: 2025-08-09T15:08:49.108Z Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
also add @author annotation to docblock
- catch InvalidLtreeException instead of InvalidArgumentException. - add docblock with reference to PostgreSQL's Ltree documentation
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/LTREE-TYPE.md (1)
38-44: Remove ORM Index attribute — it creates an invalid B-tree index for ltreeLeaving
#[ORM\Index(...)]will generate a B-tree index, which ltree does not support and will fail during migrations. The doc already shows correct GiST/GIN creation via SQL; keep only that and drop the attribute from the example.Apply:
-/** - * Manually edit `my_entity_path_gist_idx` in migration to use GIST. - * Declaring the index using Doctrine attributes prevents its removal during migrations. - */ #[ORM\Entity()] -#[ORM\Index(columns: ['path'], name: 'my_entity_path_gist_idx')] class MyEntity implements \Stringable
🧹 Nitpick comments (3)
docs/LTREE-TYPE.md (1)
208-219: Optional: avoid deep recursion when cascading path updatesLarge hierarchies could hit recursion limits in
updateChildrenPaths(). Consider iterative BFS/stack traversal.I can provide a short iterative variant if helpful.
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (1)
26-26: Suppress “unused parameter” noise (optional)
$platformis unused per interface. Prefix with_if you want to quiet linters.- public function convertToPHPValue($value, AbstractPlatform $platform): ?LtreeValueObject + public function convertToPHPValue($value, AbstractPlatform $_platform): ?LtreeValueObject ... - public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string + public function convertToDatabaseValue($value, AbstractPlatform $_platform): ?stringAlso applies to: 44-44
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (1)
190-199: Consider stricter label validation (optional)Currently checks “non-empty and no dot”. To prevent DB rejections for illegal characters, validate against the ltree label alphabet.
Example:
- if (\str_contains($value, '.')) { + if (\str_contains($value, '.') || !\preg_match('/^[A-Za-z0-9_]+$/', $value)) { throw InvalidLtreeException::forInvalidNodeFormat($value, 'string without dot'); }If quoted labels are in scope, we can extend this accordingly instead.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
README.md(2 hunks)docs/AVAILABLE-TYPES.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(4 hunks)docs/INTEGRATING-WITH-LARAVEL.md(8 hunks)docs/INTEGRATING-WITH-SYMFONY.md(4 hunks)docs/LTREE-TYPE.md(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Exceptions/InvalidLtreeException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php(1 hunks)tests/Integration/MartinGeorgiev/TestCase.php(3 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Exceptions/InvalidLtreeException.php
- src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php
- README.md
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
- docs/INTEGRATING-WITH-DOCTRINE.md
- docs/AVAILABLE-TYPES.md
- tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTypeTest.php
- tests/Integration/MartinGeorgiev/TestCase.php
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: All new features in this repository must include proper test coverage before approval - this ltree implementation demonstrates exemplary test coverage with comprehensive unit tests, integration tests, data providers for edge cases, and validation of all relationships and error conditions.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: This repository uses BaseType extension pattern for custom Doctrine DBAL types with PostgreSQL platform assertions, comprehensive unit and integration testing with data providers, and dedicated exception classes for type conversion errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.893Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
📚 Learning: 2025-08-13T05:48:00.893Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.893Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/LTREE-TYPE.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-24T16:52:32.488Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: All new features in this repository must include proper test coverage before approval - this ltree implementation demonstrates exemplary test coverage with comprehensive unit tests, integration tests, data providers for edge cases, and validation of all relationships and error conditions.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.mddocs/LTREE-TYPE.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/LTREE-TYPE.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: The PostgreSQL-for-doctrine library implements the JSON array element existence check using the `RIGHT_EXISTS_ON_LEFT` DQL function, which maps to PostgreSQL's `?` operator. This is used for checking if a specific value exists in a JSONB array column.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the point[] array type should be documented as "point[]" not "_point" in the AVAILABLE-TYPES.md table, to be consistent with all other array types like text[], jsonb[], inet[], etc.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-LARAVEL.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-03-11T17:02:51.971Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-11T17:02:51.971Z
Learning: When using operator-like functions in PostgreSQL-for-doctrine, they must be used as boolean functions with `= TRUE` rather than as direct operators (e.g., `RIGHT_EXISTS_ON_LEFT(column, value) = TRUE` instead of `column RIGHT_EXISTS_ON_LEFT value`).
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what TYPE_NAME constants contain and getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-25T21:35:48.228Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:35:48.228Z
Learning: Value object methods that call getParent() must check isEmpty() AND isRoot() to prevent LogicException - the isChildOf() method demonstrates this critical bug where root nodes crash when getParent() is called.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/LTREE-TYPE.md
📚 Learning: 2025-08-25T21:35:48.228Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:35:48.228Z
Learning: Value object methods that call getParent() must check both isEmpty() AND isRoot() conditions to prevent LogicException from being thrown when no parent exists.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-08-24T23:39:18.782Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:7-7
Timestamp: 2025-08-24T23:39:18.782Z
Learning: This repository follows a concrete value object pattern without interfaces. All value objects (Point, DateRange, Int4Range, NumericRange, etc.) are concrete classes that directly implement required interfaces like \Stringable and \JsonSerializable. The codebase uses abstract base classes for shared functionality rather than interfaces for abstraction. Adding interfaces for value objects violates the established architectural patterns.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-08-24T16:52:32.488Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: This repository uses BaseType extension pattern for custom Doctrine DBAL types with PostgreSQL platform assertions, comprehensive unit and integration testing with data providers, and dedicated exception classes for type conversion errors.
Applied to files:
docs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.mdsrc/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Applied to files:
docs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-03-26T02:46:12.804Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-03-26T02:46:12.804Z
Learning: The PR "preserve the type of floats and integers when transforming back and forth between PostgreSQL and PHP" improves type handling by ensuring that integers remain integers, floats remain floats, numeric strings stay as strings, and booleans are properly converted through the transformation process.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
docs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-04-20T11:24:18.300Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
📚 Learning: 2025-04-22T00:03:37.733Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Applied to files:
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
🧬 Code graph analysis (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Exceptions/InvalidLtreeException.php (4)
InvalidLtreeException(17-44)forImpossibleLtree(40-43)forInvalidPathFromRootFormat(22-29)forInvalidNodeFormat(31-38)src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (3)
Ltree(21-64)Override(25-41)Override(43-63)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForDatabaseException.php (3)
InvalidLtreeForDatabaseException(14-30)forInvalidFormat(26-29)forInvalidType(21-24)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php (3)
InvalidLtreeForPHPException(14-30)forInvalidFormat(26-29)forInvalidType(21-24)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (2)
Ltree(16-205)fromString(39-48)src/MartinGeorgiev/Doctrine/DBAL/Types/BaseType.php (1)
BaseType(17-52)
🪛 LanguageTool
docs/LTREE-TYPE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...//www.postgresql.org/docs/16/ltree.html) in PostgreSQL. ```sql CREATE EXTENSION ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...`` For Symfony, customize the migration that introduces ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...es the ltree field by adding this line at the beginning of the up() method: ...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...e/events.html#reference-events-on-flush) [Doctrine entity listener](https://symfo...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ...vents.html#doctrine-lifecycle-listeners) to handle updating the path column of ...
(QB_NEW_EN)
[grammar] ~154-~154: There might be a mistake here.
Context: ... column of the updated entity's children when path is present in the change set...
(QB_NEW_EN)
🪛 PHPMD (2.15.0)
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php
26-26: Avoid unused parameters such as '$platform'. (Unused Code Rules)
(UnusedFormalParameter)
44-44: Avoid unused parameters such as '$platform'. (Unused Code Rules)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: sloth
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (12)
docs/INTEGRATING-WITH-SYMFONY.md (3)
51-53: LTREE type registration looks correct
ltree: MartinGeorgiev\Doctrine\DBAL\Types\Ltreealigns with repository patterns.
118-120: Mapping type for ltree is consistent
mapping_types: ltree: ltreematches BaseType expectations and SQL declaration.
305-339: Entity usage snippet is validImport and field mapping for
Ltreeare correct.docs/INTEGRATING-WITH-LARAVEL.md (4)
86-88: Platform type mapping includes ltree — goodThe
'ltree' => 'ltree'mapping is correct.
130-132: Custom type registration for ltree is correct
'ltree' => MartinGeorgiev\Doctrine\DBAL\Types\Ltree::classmatches other scalar types.
398-401: Programmatic registration covers ltree
registerHierarchicalTypes()addsltreeproperly.
445-446: Laravel entity example is soundField mapping
#[ORM\Column(type: 'ltree')]and import are correct.Also applies to: 480-482
src/MartinGeorgiev/Doctrine/DBAL/Types/Ltree.php (3)
25-26: Verify PHP version for #[\Override] usage
#[\Override]requires PHP 8.3+. If the package supports < 8.3, drop these attributes.Apply if needed:
- #[\Override] public function convertToPHPValue($value, AbstractPlatform $platform): ?LtreeValueObject ... - #[\Override] public function convertToDatabaseValue($value, AbstractPlatform $platform): ?stringAlso applies to: 43-44
25-41: DB→PHP conversion path looks correctStrictly validates strings and maps to a domain-specific ConversionException.
44-63: PHP→DB conversion path is robustNormalizes strings via
LtreeValueObject::fromString()and handles invalid types consistently.src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (2)
36-48: Dot-splitting means no support for quoted labelsThis rejects dotted labels but allows any other chars; PostgreSQL ltree has a specific label alphabet (and quoting rules). Confirm the intended scope.
Would you like a stricter validator aligned to the ltree label grammar, or to add quoted-label support?
142-149: Sibling semantics: roots become siblings via empty parent
isSiblingOf()treats two single-node paths as siblings since both parents are empty. If that’s not desired, add a guard forisRoot().I can propose a small guard if you prefer different semantics.
Contributor
Author
|
@martin-georgiev This should be OK. I'm available until sunday to fix issues. |
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
Outdated
Show resolved
Hide resolved
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
Outdated
Show resolved
Hide resolved
rename `throws_exception_when_getting_empty_ltree_parent()` to `throws_exception_when_ltree_is_empty_and_therefore_has_no_parent()`.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (5)
56-60: Add a couple more invalid-string edge cases.Strengthen negative coverage with consecutive dots and dot-only inputs.
Apply:
public static function provideInvalidStringRepresentation(): iterable { yield 'string starting with dot' => ['.b']; yield 'string ending with dot' => ['a.']; + yield 'string with consecutive dots' => ['a..b']; + yield 'single dot only' => ['.']; + yield 'string with multiple consecutive dots' => ['a...b']; }
111-117: Also assert JSON encoding for an empty ltree.Covers the empty-path serialization contract.
Apply:
public function can_encode_to_json_array(): void { $ltree = new Ltree(['a', 'b', 'c']); $json = \json_encode($ltree, \JSON_THROW_ON_ERROR); $this->assertSame('["a","b","c"]', $json); } + + #[Test] + public function can_encode_empty_ltree_to_json_array(): void + { + $ltree = new Ltree([]); + $json = \json_encode($ltree, \JSON_THROW_ON_ERROR); + $this->assertSame('[]', $json); + }
127-135: Use $parent to avoid PHPMD “UnusedFormalParameter” and tighten the assertion.Leverage the provided expected parent in the immutability test.
Apply:
public function respect_immutability_when_getting_parent(Ltree $child, Ltree $parent): void { $childAsString = (string) $child; $ltree = $child->getParent(); $this->assertNotSame($child, $ltree, 'getParent() should return a new instance'); $this->assertSame($childAsString, (string) $child, 'getParent() should not mutate the original instance'); + $this->assertSame((string) $parent, (string) $ltree, 'getParent() should return the expected parent'); }
129-129: Naming consistency: prefer “respects_immutability_…” (plural) to match the leaf test.Minor style alignment only.
485-493: Use $expected to avoid PHPMD “UnusedFormalParameter” and assert the result.Keeps the immutability test self-sufficient while using all inputs.
Apply:
public function respects_immutability_when_creating_leaf(Ltree $parent, string $leaf, Ltree $expected): void { $parentAsString = (string) $parent; $ltree = $parent->withLeaf($leaf); + $this->assertSame((string) $expected, (string) $ltree, 'withLeaf() should produce the expected instance'); $this->assertNotSame($parent, $ltree, 'withLeaf() should return a new instance'); $this->assertSame($parentAsString, (string) $parent, 'withLeaf() should not mutate the original instance'); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: All new features in this repository must include proper test coverage before approval - this ltree implementation demonstrates exemplary test coverage with comprehensive unit tests, integration tests, data providers for edge cases, and validation of all relationships and error conditions.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: This repository uses BaseType extension pattern for custom Doctrine DBAL types with PostgreSQL platform assertions, comprehensive unit and integration testing with data providers, and dedicated exception classes for type conversion errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.893Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#352
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php:1-67
Timestamp: 2025-04-20T11:24:18.300Z
Learning: This PostgreSQL-for-Doctrine project is a translation layer only, focusing on correctly converting Doctrine DQL to PostgreSQL SQL syntax. It ensures arguments are passed in the expected format but does not test or handle PostgreSQL's actual function behavior or data handling. Test cases should focus on DQL-to-SQL translation and argument validation, not on PostgreSQL-specific behaviors.
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
📚 Learning: 2025-08-24T16:52:32.488Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: All new features in this repository must include proper test coverage before approval - this ltree implementation demonstrates exemplary test coverage with comprehensive unit tests, integration tests, data providers for edge cases, and validation of all relationships and error conditions.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-08-13T05:48:00.893Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: docs/LTREE-TYPE.md:72-77
Timestamp: 2025-08-13T05:48:00.893Z
Learning: In the ltree implementation, the Ltree class has built-in validation and doesn't accept invalid values. An empty branch [] is a valid ltree path. The setParent() method in ltree-based entities ensures there are no circular references, making constructor calls to setParent() safe.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-04-17T12:29:38.149Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-04-17T12:29:38.149Z
Learning: All new features in this repository must include proper test coverage before approval.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-08-25T21:35:48.228Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:35:48.228Z
Learning: Value object methods that call getParent() must check isEmpty() AND isRoot() to prevent LogicException - the isChildOf() method demonstrates this critical bug where root nodes crash when getParent() is called.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-08-25T21:35:48.228Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:35:48.228Z
Learning: Value object methods that call getParent() must check both isEmpty() AND isRoot() conditions to prevent LogicException from being thrown when no parent exists.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
📚 Learning: 2025-08-09T15:08:49.108Z
Learnt from: landure
PR: martin-georgiev/postgresql-for-doctrine#411
File: tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/LtreeTest.php:58-66
Timestamp: 2025-08-09T15:08:49.108Z
Learning: PostgreSQL's `ltree` extension accepts empty strings (`''`) as valid input, representing a path with no labels. This is valid in PostgreSQL 17 and likely earlier versions as well. Empty ltree values can be inserted and stored without syntax errors.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
🧬 Code graph analysis (1)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Exceptions/InvalidLtreeException.php (1)
InvalidLtreeException(17-44)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Ltree.php (7)
Ltree(16-205)fromString(39-48)getPathFromRoot(62-65)getParent(70-79)isEmpty(81-84)isRoot(89-92)withLeaf(158-165)
🪛 PHPMD (2.15.0)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php
129-129: Avoid unused parameters such as '$parent'. (Unused Code Rules)
(UnusedFormalParameter)
487-487: Avoid unused parameters such as '$expected'. (Unused Code Rules)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: sloth
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (1)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeTest.php (1)
219-469: Relationship matrix coverage is excellent.Thorough cases across empty/root/child/sibling/unrelated; semantics match the value object’s implementation.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ltreevalue objectLtreedoctrine typefix #410
Summary by CodeRabbit
New Features
Bug Fixes / Errors
Documentation
Tests