feat(#305): add support for PostGIS operators #430
Conversation
WalkthroughAdds a spatial fixture entity, many new Doctrine DQL AST function classes implementing PostGIS bbox/strict/overlap/distance/trajectory/n‑D/containment operators, extends unit tests for geometry/geography cases, and introduces an integration test base that creates and populates a spatial fixture table with numerous integration tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (writes DQL)
participant Parser as Doctrine DQL Parser
participant AST as AST Function (e.g. OverlapsRight / GeometryDistance)
participant SQL as SQL Generator
participant DB as PostgreSQL+PostGIS
Dev->>Parser: SELECT OVERLAPS_RIGHT(e.geometry1, e.geometry2)
Parser->>AST: parse function node with operands
note right of AST #E8F0FF: prototype = '(%s &> %s)'\noperands → StringPrimary
AST-->>SQL: emit SQL fragment "(c0_.geometry1 &> c0_.geometry2)"
SQL->>DB: execute SQL
DB-->>SQL: boolean result
SQL-->>Dev: scalar boolean
Dev->>Parser: SELECT GEOMETRY_DISTANCE(e.geometry1, e.geometry2)
Parser->>AST: parse function node with operands
note right of AST #FFF4E6: prototype = '(%s <-> %s)'\noperands → StringPrimary
AST-->>SQL: emit SQL fragment "(colA <-> colB)"
SQL->>DB: execute SQL
DB-->>SQL: numeric result
SQL-->>Dev: scalar numeric
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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: 0
🧹 Nitpick comments (15)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.php (2)
24-25: Use a valid polygon WKT to avoid ambiguity in examplesThe literal
POLYGON((0 0, 2 2, 4 4, 0 0))has collinear vertices and zero area; while these tests assert SQL generation (not execution), a valid polygon improves clarity and future-proofing if tests evolve to integration. Suggest a simple triangle.- 'checks if geometry is contained by literal' => "SELECT (c0_.geometry1 <@ 'POLYGON((0 0, 2 2, 4 4, 0 0))') AS sclr_0 FROM ContainsGeometries c0_", + 'checks if geometry is contained by literal' => "SELECT (c0_.geometry1 <@ 'POLYGON((0 0, 2 0, 0 2, 0 0))') AS sclr_0 FROM ContainsGeometries c0_", @@ - 'checks if geometry is contained by literal' => \sprintf("SELECT IS_CONTAINED_BY(e.geometry1, 'POLYGON((0 0, 2 2, 4 4, 0 0))') FROM %s e", ContainsGeometries::class), + 'checks if geometry is contained by literal' => \sprintf("SELECT IS_CONTAINED_BY(e.geometry1, 'POLYGON((0 0, 2 0, 0 2, 0 0))') FROM %s e", ContainsGeometries::class),Also applies to: 33-34
20-26: Optional: add a parameterized geometry caseMirroring the array parameter case would exercise SQL generation with bound parameters for spatial values as well. Example key: "contained by using parameter".
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (2)
24-25: Swap invalid polygon WKT for a simple valid polygonCurrent WKT
POLYGON((0 0, 1 1, 2 2, 0 0))is degenerate (collinear vertices). Replace with a valid triangle to avoid confusion.- 'checks if geometry overlaps literal' => "SELECT (c0_.geometry1 && 'POLYGON((0 0, 1 1, 2 2, 0 0))') AS sclr_0 FROM ContainsGeometries c0_", + 'checks if geometry overlaps literal' => "SELECT (c0_.geometry1 && 'POLYGON((0 0, 2 0, 0 2, 0 0))')') AS sclr_0 FROM ContainsGeometries c0_", @@ - 'checks if geometry overlaps literal' => \sprintf("SELECT OVERLAPS(e.geometry1, 'POLYGON((0 0, 1 1, 2 2, 0 0))') FROM %s e", ContainsGeometries::class), + 'checks if geometry overlaps literal' => \sprintf("SELECT OVERLAPS(e.geometry1, 'POLYGON((0 0, 2 0, 0 2, 0 0))') FROM %s e", ContainsGeometries::class),Also applies to: 33-34
20-26: Optional: consider 3D overlaps (&&&) coverage in a follow-upIf/when 3D types are in scope, adding a counterpart test for
&&&would round out spatial overlap operators.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.php (1)
20-29: Optional: add geography field coverage and a parameterized geometry case
- Geography columns exist in the fixture; a couple of assertions would ensure identical translation on that type.
- A
:parameter-backed geometry case (similar to arrays) would validate parameter handling for spatial values.Also applies to: 32-41
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (1)
21-24: Optional: add a reversed-literal case and boolean-usage example
- Consider also asserting SQL for a literal on the left (
'POINT(...)' &< e.geometry2) to ensure both operand orderings are parsable.- A separate DQL example using
= TRUEin WHERE (even as a comment or additional fixture) would document the intended boolean usage pattern.Also applies to: 29-32
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
7-19: Docblock: clarify geometry-only semantics and verify @SInCE version.These directional bbox operators are defined for geometry in PostGIS. Consider stating that explicitly, and please confirm the correct library version for
@since.Apply this doc-only tweak:
/** - * Implementation of PostGIS bounding box overlaps or is below operator (using &<|). + * Implementation of the PostGIS geometry bounding-box “overlaps or is below” operator (&<|). * - * Returns TRUE if A's bounding box overlaps or is below B's. + * Returns TRUE if A's bounding box overlaps or is below B's. Works with geometry columns. + * Note: PostGIS defines these directional bbox operators for geometry (not geography). * * @see https://postgis.net/docs/reference.html#Operators_Geometry - * @since 3.5 + * @since 3.5 // verify before merge * * @author Martin Georgiev <martin.georgiev@gmail.com> * * @example Using it in DQL with boolean comparison: "WHERE OVERLAPS_BELOW(g1.geometry, g2.geometry) = TRUE" * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. */src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
7-19: Docblock parity with other operators + version check.Mirror the geometry-only note and double-check
@since 3.5is the intended release tag for this addition.Proposed doc tweak:
/** - * Implementation of PostGIS bounding box overlaps or is to the right operator (using &>). + * Implementation of the PostGIS geometry bounding-box “overlaps or is to the right” operator (&>). * - * Returns TRUE if A's bounding box overlaps or is to the right of B's. + * Returns TRUE if A's bounding box overlaps or is to the right of B's. Works with geometry columns. + * Note: PostGIS defines these directional bbox operators for geometry (not geography). * * @see https://postgis.net/docs/reference.html#Operators_Geometry - * @since 3.5 + * @since 3.5 // verify before mergesrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
7-19: Docblock: add geometry-only note and confirm version.Keep docs consistent across the new operators and validate the
@sinceannotation.Suggested small doc refactor:
/** - * Implementation of PostGIS bounding box overlaps or is above operator (using |&>). + * Implementation of the PostGIS geometry bounding-box “overlaps or is above” operator (|&>). * - * Returns TRUE if A's bounding box overlaps or is above B's. + * Returns TRUE if A's bounding box overlaps or is above B's. Works with geometry columns. + * Note: PostGIS defines these directional bbox operators for geometry (not geography). * * @see https://postgis.net/docs/reference.html#Operators_Geometry - * @since 3.5 + * @since 3.5 // verify before mergetests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (1)
19-24: Add literal-operand coverage for parity with OverlapsLeftTest.OverlapsLeftTest asserts both column–column and column–literal forms. Suggest adding the literal case here too to lock in quoting and walker behavior.
Apply:
protected function getExpectedSqlStatements(): array { return [ - 'checks if geometry overlaps or is to the right' => 'SELECT (c0_.geometry1 &> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'checks if geometry overlaps or is to the right' => 'SELECT (c0_.geometry1 &> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'checks if geometry overlaps or is to the right of literal' => "SELECT (c0_.geometry1 &> 'POINT(1 2)') AS sclr_0 FROM ContainsGeometries c0_", ]; } protected function getDqlStatements(): array { return [ - 'checks if geometry overlaps or is to the right' => \sprintf('SELECT OVERLAPS_RIGHT(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'checks if geometry overlaps or is to the right' => \sprintf('SELECT OVERLAPS_RIGHT(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'checks if geometry overlaps or is to the right of literal' => \sprintf("SELECT OVERLAPS_RIGHT(e.geometry1, 'POINT(1 2)') FROM %s e", ContainsGeometries::class), ]; }Also applies to: 26-31
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
13-23: Column nullability should match property types.Properties are
?WktSpatialDatabut columns don’t declarenullable: true. It’s fine for query-only fixtures, but adding nullability avoids confusion and future persistence issues.Apply:
- #[ORM\Column(type: 'geometry')] + #[ORM\Column(type: 'geometry', nullable: true)] public ?WktSpatialData $geometry1 = null; - #[ORM\Column(type: 'geometry')] + #[ORM\Column(type: 'geometry', nullable: true)] public ?WktSpatialData $geometry2 = null; - #[ORM\Column(type: 'geography')] + #[ORM\Column(type: 'geography', nullable: true)] public ?WktSpatialData $geography1 = null; - #[ORM\Column(type: 'geography')] + #[ORM\Column(type: 'geography', nullable: true)] public ?WktSpatialData $geography2 = null;tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (2)
19-24: Operator and SQL expectation look right; small resilience nit.The SQL uses the PostGIS bbox operator |&> with parentheses, which is correct for precedence. Minor: asserting the exact SQL including alias "c0_" can be brittle across Doctrine versions. If you ever bump Doctrine, consider a helper that normalizes/ignores auto-generated aliases.
26-31: Add a reverse-operand case to validate directionality.These operators are directional. Adding a second example with swapped operands will guard against accidental operand order regressions in the DQL function.
Apply this diff to extend both arrays:
@@ protected function getExpectedSqlStatements(): array { return [ 'checks if geometry overlaps or is above' => 'SELECT (c0_.geometry1 |&> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'reversed operands (directionality check)' => 'SELECT (c0_.geometry2 |&> c0_.geometry1) AS sclr_0 FROM ContainsGeometries c0_', ]; } @@ protected function getDqlStatements(): array { return [ 'checks if geometry overlaps or is above' => \sprintf('SELECT OVERLAPS_ABOVE(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'reversed operands (directionality check)' => \sprintf('SELECT OVERLAPS_ABOVE(e.geometry2, e.geometry1) FROM %s e', ContainsGeometries::class), ]; }tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (2)
19-24: Expected SQL uses the correct &<| operator; tiny note on alias brittleness.Parentheses and operator are correct. Same minor note: exact alias "c0_" can change with Doctrine. If you later upgrade, consider normalizing generated aliases in assertions.
26-31: Add a directionality check with swapped operands.Mirrors the suggestion for OVERLAPS_ABOVE to ensure the operand order is honored.
Apply this diff:
@@ protected function getExpectedSqlStatements(): array { return [ 'checks if geometry overlaps or is below' => 'SELECT (c0_.geometry1 &<| c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'reversed operands (directionality check)' => 'SELECT (c0_.geometry2 &<| c0_.geometry1) AS sclr_0 FROM ContainsGeometries c0_', ]; } @@ protected function getDqlStatements(): array { return [ 'checks if geometry overlaps or is below' => \sprintf('SELECT OVERLAPS_BELOW(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'reversed operands (directionality check)' => \sprintf('SELECT OVERLAPS_BELOW(e.geometry2, e.geometry1) FROM %s e', ContainsGeometries::class), ]; }
📜 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 (12)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.php(3 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.php(2 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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/ORM/Query/AST/Functions/OverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.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/ORM/Query/AST/Functions/OverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.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/ORM/Query/AST/Functions/OverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.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/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.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/ORM/Query/AST/Functions/OverlapsLeft.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.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/ORM/Query/AST/Functions/OverlapsRight.php
🧬 Code graph analysis (11)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (6)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.php (1)
getDqlStatements(32-42)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.php (1)
getDqlStatements(29-36)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (1)
getDqlStatements(26-31)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (1)
getDqlStatements(26-31)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (1)
getDqlStatements(26-31)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
customizeFunction(22-27)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
OverlapsRight(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (3)
getStringFunctions(12-17)getExpectedSqlStatements(19-24)getDqlStatements(26-31)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (3)
getStringFunctions(12-17)getExpectedSqlStatements(19-24)getDqlStatements(26-31)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (3)
getStringFunctions(12-17)getExpectedSqlStatements(19-25)getDqlStatements(27-33)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.php (2)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.php (1)
getDqlStatements(32-42)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (1)
getDqlStatements(29-36)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (5)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php (1)
Overlaps(18-26)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
OverlapsAbove(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (2)
getExpectedSqlStatements(19-24)getDqlStatements(26-31)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (2)
getExpectedSqlStatements(19-24)getDqlStatements(26-31)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (2)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (1)
OverlapsLeft(20-28)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (5)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php (1)
Overlaps(18-26)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (2)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
OverlapsBelow(20-28)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/WktSpatialData.php (1)
WktSpatialData(22-109)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
customizeFunction(22-27)
⏰ 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: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + 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 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + 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.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + 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 2.18 + Doctrine Lexer 3.0
- 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 latest + Doctrine Lexer 2.1
- GitHub Check: sloth
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (15)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (2)
22-27: Operator prototype and node mapping mirror existing overlaps variants — LGTMMatches the pattern used by OverlapsAbove/Below/Right, uses StringPrimary on both operands, and emits the expected
(%s &< %s)SQL. Good consistency and minimal surface area.
7-19: Verify the@since 3.5annotation before mergeEnsure the version tag aligns with the planned next release; if the library will cut 3.x with a different minor, update the tag accordingly to avoid churn in generated docs.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/IsContainedByTest.php (1)
8-8: Good: adds geometry fixture to broaden coverageImporting
ContainsGeometrieskeeps tests cohesive with other spatial operator tests in this PR.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (1)
8-8: Good: imports geometry fixture and expands coverage to spatial overlapsKeeps parity with array tests and aligns with production operator semantics (
&&).tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ContainsTest.php (1)
27-28: LGTM: spatial contains test cases added consistentlyAdds geometry-to-geometry and geometry-to-literal for
@>matching the DQL mapping. Consistent with the new fixture.Also applies to: 39-40
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (1)
12-17: Function mapping looks correct
OVERLAPS_LEFT→OverlapsLeft::classmatches production name and mirrors other directional overlaps tests.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
22-27: Operator mapping and parser nodes look correct and consistent with existing Overlaps.*Prototype, operands, and parentheses mirror the pattern used in Overlaps/Left/Right/Above.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
22-27: LGTM: correct prototype and node mapping.Matches the established pattern; SQL generation should be fine.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
22-27: Consistent implementation of |&> operator.Prototype and node mappings are in line with the rest of the Overlaps* family.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (3)
5-11: Extends the correct project TestCase; namespace and imports look consistent.Matches the established testing pattern in this repo (custom TestCase in this namespace; no explicit import needed).
12-17: String function mapping is correct and consistent.'OVERLAPS_ABOVE' → OverlapsAbove::class aligns with the production class name and casing used elsewhere.
7-9: Confirm whether geography columns should be covered too.The fixture provides geography1/geography2; if PostGIS supports these bbox operators for geography in your target versions, consider adding a geography variant test. If not, ignore this.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (3)
5-11: Consistent setup with the project TestCase; good.Namespace, imports, and class declaration follow the repository convention.
12-17: Correct DQL function wiring.'OVERLAPS_BELOW' → OverlapsBelow::class matches production naming.
7-9: Consider geography coverage if applicable.Same as ABOVE: if bbox operators are valid for geography in your targeted PostGIS versions, add a geography pair; otherwise skip.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (35)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (4)
22-24: Consider avoiding re-creating the entire schema on every test methodCalling createTestSchema() here means every test method drops/recreates the schema (and tries to move the PostGIS extension). For large suites this adds overhead and can introduce cross-test flakiness if other tests rely on the same schema lifecycle.
You can:
- Move createTestSchema() to BaseTestCase setUp (if not already there) or a @beforeClass hook in this abstract test to run once per class.
- Keep the per-test table drop/create and inserts for isolation.
Do you want a patch sketch for a @beforeClass-based setup?
28-34: Prefer IDENTITY over SERIAL for new DDLSERIAL is legacy; PostgreSQL 10+ recommends IDENTITY columns. This avoids implicit sequences and plays nicer with dumps/restore.
Apply:
- id SERIAL PRIMARY KEY, + id INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
45-64: Add measured (M) linestrings to better exercise trajectory-aware operatorsThe |=| operator computes “closest point of approach” distance between trajectories and expects valid M-measured lines (see PostGIS docs). While unit tests only check SQL translation, adding at least one measured pair in the fixture lets integration tests query column-to-column CPA directly, not only literals. (postgis.net, access.crunchydata.com)
Minimal change (safe for bbox predicates too):
- 'geometry1' => 'LINESTRING(0 0, 1 1, 2 2)', - 'geometry2' => 'LINESTRING(3 3, 4 4, 5 5)', + 'geometry1' => 'LINESTRING M(0 0 0, 1 1 1, 2 2 2)', + 'geometry2' => 'LINESTRING M(3 3 0, 4 4 1, 5 5 2)',PostGIS accepts both WKT2 “LINESTRING M(…)” and EWKT “LINESTRINGM(…)” notations. (postgis.net)
If you want, I can scan the suite to confirm no tests assume strictly 2D lines.
66-78: Reuse a prepared statement instead of rebuilding SQL inside the loopMinor, but avoids re-allocating the SQL string and prepares once.
Apply within this block:
- foreach ($testData as $row) { - $sql = \sprintf(' - INSERT INTO %s (geometry1, geometry2, geography1, geography2) - VALUES (ST_GeomFromText(?), ST_GeomFromText(?), ST_GeogFromText(?), ST_GeogFromText(?)) - ', $fullTableName); - - $this->connection->executeStatement($sql, [ + $sql = \sprintf(' + INSERT INTO %s (geometry1, geometry2, geography1, geography2) + VALUES (ST_GeomFromText(?), ST_GeomFromText(?), ST_GeogFromText(?), ST_GeogFromText(?)) + ', $fullTableName); + foreach ($testData as $row) { + $this->connection->executeStatement($sql, [ $row['geometry1'], $row['geometry2'], $row['geography1'], $row['geography2'], ]); }Alternatively, prepare once for clarity:
$sql = sprintf('INSERT INTO %s (geometry1, geometry2, geography1, geography2) VALUES (ST_GeomFromText(?), ST_GeomFromText(?), ST_GeogFromText(?), ST_GeogFromText(?))', $fullTableName); $stmt = $this->connection->prepare($sql); foreach ($testData as $row) { $stmt->executeStatement([$row['geometry1'], $row['geometry2'], $row['geography1'], $row['geography2']]); }tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (1)
26-31: Add a literal-vs-column and literal-vs-literal case to harden translation coverageSince |=| activates ND-KNN behavior mainly in ORDER BY contexts and supports literals, adding one more DQL/SQL pair with a measured literal would broaden coverage (even if unit tests don’t execute). (postgis.net)
Example additions:
protected function getExpectedSqlStatements(): array { return [ 'calculates trajectory distance between geometries' => 'SELECT (c0_.geometry1 |=| c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'trajectory distance column vs literal' => "SELECT (c0_.geometry1 |=| 'LINESTRING M(0 0 0, 1 1 1)') AS sclr_0 FROM ContainsGeometries c0_", ]; } protected function getDqlStatements(): array { return [ 'calculates trajectory distance between geometries' => \sprintf('SELECT TRAJECTORY_DISTANCE(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'trajectory distance column vs literal' => \sprintf("SELECT TRAJECTORY_DISTANCE(e.geometry1, 'LINESTRING M(0 0 0, 1 1 1)') FROM %s e", ContainsGeometries::class), ]; }src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
17-19: Doc nit: DQL usage note can be clearerDoctrine on PostgreSQL supports boolean expressions directly in WHERE. The example with “= TRUE” is fine, but consider also showing direct usage:
- WHERE STRICTLY_BELOW(g1.geometry, g2.geometry)
I can update the docblock across all operator classes for consistency if you’d like.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (1)
26-31: Optional: add a literal case to mirror the integration testsParallels your integration scenario where one operand is a WKT literal.
Proposed additions:
protected function getExpectedSqlStatements(): array { return [ 'checks if geometry is strictly to the right' => 'SELECT (c0_.geometry1 >> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'strictly right with literal geometry' => "SELECT (c0_.geometry1 >> 'POINT(-5 -5)') AS sclr_0 FROM ContainsGeometries c0_", ]; } protected function getDqlStatements(): array { return [ 'checks if geometry is strictly to the right' => \sprintf('SELECT STRICTLY_RIGHT(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'strictly right with literal geometry' => \sprintf("SELECT STRICTLY_RIGHT(e.geometry1, 'POINT(-5 -5)') FROM %s e", ContainsGeometries::class), ]; }src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php (2)
7-20: Clarify operand requirements and link precision in the docblockTighten the phpdoc to set expectations (linear geometries with M, geometry type only), and point at the exact operator section in the PostGIS docs. Also call out the return type as double precision for clarity in consumers/tests.
Apply this doc-only diff:
-/** - * Implementation of PostGIS trajectory distance operator (using |=|). - * - * Returns the distance between A and B trajectories at their closest point of approach. - * Trajectories are linear geometries with increasing measures (M value) on each coordinate. - * - * @see https://postgis.net/docs/reference.html#Operators_Distance - * @since 3.5 - * - * @author Martin Georgiev <martin.georgiev@gmail.com> - * - * @example Using it in DQL: "SELECT TRAJECTORY_DISTANCE(t1.trajectory, t2.trajectory) FROM Entity t1, Entity t2" - * Returns numeric distance value. - */ +/** + * Implementation of PostGIS trajectory distance operator (|=|). + * + * Returns the distance between A and B trajectories at their closest point of approach. + * Operands should be linear geometries (e.g., LINESTRING) with increasing M values (measures). + * Operates on geometry; geography is not supported by this operator. + * + * @see https://postgis.net/docs/reference.html#Operators_Distance // includes |=| + * @since 3.5 + * + * @author Martin Georgiev <martin.georgiev@gmail.com> + * + * @example DQL: SELECT TRAJECTORY_DISTANCE(t1.trajectory, t2.trajectory) FROM Entity t1, Entity t2 + * @return double precision + */
21-29: Consider a tiny helper for binary spatial operators to reduce repetition across new classesMany new operators share the same structure (two StringPrimary nodes + prototype). A small reusable helper/trait (e.g., BinarySpatialOperator::configure('(%s |=| %s)')) would keep implementations DRY and make future additions harder to get wrong.
If helpful, I can draft a minimal trait and show how these classes would consume it.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php (1)
26-31: Optional: add a second assertion covering geography columns or parametersWhile the operator targets geometry, a parametric DQL case (using a bound WKT parameter casted appropriately) or an explicit geometry column reference would broaden coverage beyond class name resolution.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
7-19: Tighten DQL usage guidance in the docblockDoctrine versions differ in how boolean-returning functions can be used directly in WHERE vs. requiring explicit comparison. To avoid confusion, suggest clarifying that explicit comparison is the recommended cross-version approach and that the operator applies to geometry bboxes.
Suggested doc tweak:
- * @example Using it in DQL with boolean comparison: "WHERE STRICTLY_LEFT(g1.geometry, g2.geometry) = TRUE" - * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. + * @example Recommended DQL usage (cross-version): WHERE STRICTLY_LEFT(g1.geometry, g2.geometry) = TRUE + * Returns boolean; explicit TRUE/FALSE comparison is recommended for portability across Doctrine versions. + * Operates on geometry bounding boxes.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
7-20: Document geometry-only scope and relation to <-> for clarityReaders often conflate <#> with <->. A short note helps set expectations and guides operator choice.
Doc-only diff:
- * Returns the 2D distance between A and B bounding boxes. - * This is useful for index-based distance queries. + * Returns the 2D distance between A and B bounding boxes (geometry only). + * For KNN index queries, <-> is commonly used; <#> provides an alternative bbox distance metric.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php (3)
19-29: Literal WKT without explicit casts can be version-sensitiveUsing string literals like 'POINT(1 1)' relies on Postgres resolving “unknown” literals to geometry via implicit typing. This can vary (and some setups require explicit casts). If CI covers your supported matrix and passes, great; otherwise, consider explicitly casting or using geometry columns to avoid implicit-cast reliance.
Two low-impact alternatives:
- Use columns for “zero distance” by selecting the same column twice, e.g., BOUNDING_BOX_DISTANCE(g.geometry1, g.geometry1).
- If available in your DQL setup, wrap as CAST('POINT(1 1)' AS geometry) or ST_GeomFromText('POINT(1 1)') to be explicit.
39-41: Make numeric assertions resilient to driver return types (string vs float)DBAL may return numeric scalars as strings. Casting to float (or using with-delta assertions) keeps tests stable across platforms.
Apply this focused diff:
- $this->assertEquals(0, $result[0]['distance'], 'Bounding box distance for identical geometries should be 0'); + $this->assertSame(0.0, (float) $result[0]['distance'], 'Bounding box distance for identical geometries should be 0'); - $this->assertEquals(0, $result[0]['distance'], 'Overlapping bounding boxes should have distance 0'); + $this->assertSame(0.0, (float) $result[0]['distance'], 'Overlapping bounding boxes should have distance 0'); - $this->assertEquals(5.0, $result[0]['distance'], 'Bounding box distance between (0,0) and (3,4) should be 5'); + $this->assertEqualsWithDelta(5.0, (float) $result[0]['distance'], 1e-9, 'Bounding box distance between (0,0) and (3,4) should be ~5'); - $this->assertEqualsWithDelta(2.83, $result[0]['distance'], 0.1, 'Distance between separated polygons'); + $this->assertEqualsWithDelta(2.83, (float) $result[0]['distance'], 0.1, 'Distance between separated polygons');Also applies to: 51-53, 63-68
31-41: Optional: add a column-based zero-distance caseThis complements the literal-WKT cases and exercises translation with entity fields. For example, seed a row with identical geometry1/geometry2 or simply reference the same column twice.
I can draft an additional test method using BOUNDING_BOX_DISTANCE(g.geometry1, g.geometry1) if you’d like.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
7-19: Docblock: align usage note with Doctrine boolean semantics and refresh links (@see).
- The operator returns a boolean and can be used directly in DQL WHERE clauses without explicit “= TRUE/= FALSE”. The explicit comparison is optional. Let’s tweak the example and wording for consistency with the rest of the operators. Also, prefer pointing @see to the current PostGIS Operators section and the dedicated |>> page for precision.
Apply this doc-only diff:
/** * Implementation of PostGIS bounding box strictly above operator (using |>>). * * Returns TRUE if A's bounding box is strictly above B's. * - * @see https://postgis.net/docs/reference.html#Operators_Geometry + * @see https://postgis.net/docs/reference.html#Operators + * @see https://postgis.net/docs/ST_Geometry_Above.html * @since 3.5 * * @author Martin Georgiev <martin.georgiev@gmail.com> * - * @example Using it in DQL with boolean comparison: "WHERE STRICTLY_ABOVE(g1.geometry, g2.geometry) = TRUE" - * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. + * @example DQL WHERE usage: "WHERE STRICTLY_ABOVE(g1.geometry, g2.geometry)" + * @example DQL SELECT usage: "SELECT STRICTLY_ABOVE(g1.geometry, g2.geometry) AS result FROM Entity g1, Entity g2" + * Returns a boolean. Explicit comparisons (= TRUE / = FALSE) are optional in DQL. */For reference, PostGIS documents |>> as “A strictly above B” under Bounding Box Operators. (postgis.net, postgis.net)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (2)
19-24: Add a literal-geometry case to unit SQL expectations for symmetry and coverage.Unit tests elsewhere include literal geometry arguments; mirroring that here will lock in the SQL shape for that variant.
Apply this minimal diff:
protected function getExpectedSqlStatements(): array { return [ 'checks if geometry is strictly below' => 'SELECT (c0_.geometry1 <<| c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'checks if geometry is strictly below (literal RHS)' => 'SELECT (c0_.geometry1 <<| \'POINT(0 -2)\') AS sclr_0 FROM ContainsGeometries c0_', ]; }The <<| operator for “strictly below” is documented by PostGIS. (postgis.net)
26-31: Mirror the DQL for the new literal-geometry unit check.protected function getDqlStatements(): array { return [ 'checks if geometry is strictly below' => \sprintf('SELECT STRICTLY_BELOW(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'checks if geometry is strictly below (literal RHS)' => \sprintf('SELECT STRICTLY_BELOW(e.geometry1, \'POINT(0 -2)\') FROM %s e', ContainsGeometries::class), ]; }tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (3)
19-28: Assert actual truth-values, not just type, for a stronger signal.Given the fixture data:
- Row id=2 has geometry1 below geometry2, so STRICTLY_ABOVE(geometry1, geometry2) should be false.
Replacing the type-only assertion with an exact boolean check catches directionality regressions.Apply this diff:
- $result = $this->executeDqlQuery($dql); - $this->assertIsBool($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertFalse($result[0]['result']);PostGIS defines |>> as “A is strictly above B” (A.minY > B.maxY). (postgis.net)
30-39: Lock in a positive case using a literal geometry.For row id=1, geometry1 is POINT(0 0) and 'POINT(0 -2)' lies strictly below it, so STRICTLY_ABOVE(geometry1, 'POINT(0 -2)') should be true. This also validates the literal input path.
- $result = $this->executeDqlQuery($dql); - $this->assertIsBool($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']);Note: Postgres/PostGIS typically provides an implicit cast from text/unknown WKT to geometry; relying on it is okay, but parameterizing with a geometry-typed value is even clearer. (postgis.net)
41-50: Add an explicit true-case for linestrings.Row id=3 places geometry2 fully above geometry1, so STRICTLY_ABOVE(geometry2, geometry1) should be true.
- $result = $this->executeDqlQuery($dql); - $this->assertIsBool($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']);Definition reference for |>>. (postgis.net)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (3)
19-28: Tighten assertions to verify operator direction (expected false here).For id=1, POINT(0 0) is not strictly to the right of POINT(1 1), so the result should be false.
- $result = $this->executeDqlQuery($dql); - $this->assertIsBool($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertFalse($result[0]['result']);The >> operator is “A strictly to the right of B.” (postgis.net)
30-39: Strengthen the literal-geometry case by asserting true.'POINT(-5 -5)' lies strictly to the left of POINT(0 0), hence geometry1 >> literal should be true.
- $result = $this->executeDqlQuery($dql); - $this->assertIsBool($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']);Operator reference for >>. (postgis.net)
41-50: Assert true for the linestring comparison.For id=3, geometry2 is entirely to the right of geometry1, so the expression should evaluate to true.
- $result = $this->executeDqlQuery($dql); - $this->assertIsBool($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']);PostGIS >> semantics. (postgis.net)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (2)
10-11: Add explicit table mapping to decouple from naming strategy.Integration tests create a physical table named "containsgeometries" (lowercase). Relying on default naming may work due to Postgres lowercasing unquoted identifiers, but an explicit mapping is safer and self-documenting.
Apply:
-#[ORM\Entity()] -class ContainsGeometries extends Entity +#[ORM.Entity()] +#[ORM\Table(name: 'containsgeometries')] +class ContainsGeometries extends Entity
13-23: Confirm usage pattern for uninitialized typed properties.Public, non-nullable typed properties without defaults are fine for read-only fixtures hydrated by Doctrine. If this entity is ever persisted without explicitly setting these fields, PHP will throw "Typed property must not be accessed before initialization."
If you expect any future writes, consider making these nullable with
nullable: trueand initializing tonull, or adding a minimal constructor that sets sane defaults viaWktSpatialData::fromWkt(). Happy to provide a patch if you want to go that route.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (1)
19-25: Add symmetric literal-on-the-left case to harden coverage.You cover column vs column and column vs literal. Add literal vs column to ensure node mapping accepts a literal as the first operand too.
Apply:
return [ 'checks if geometry is strictly to the left' => 'SELECT (c0_.geometry1 << c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', 'checks if geometry is strictly to the left of literal' => "SELECT (c0_.geometry1 << 'POINT(1 2)') AS sclr_0 FROM ContainsGeometries c0_", + 'checks if literal is strictly to the left of geometry' => "SELECT ('POINT(1 2)' << c0_.geometry1) AS sclr_0 FROM ContainsGeometries c0_", ];And:
return [ 'checks if geometry is strictly to the left' => \sprintf('SELECT STRICTLY_LEFT(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), 'checks if geometry is strictly to the left of literal' => \sprintf("SELECT STRICTLY_LEFT(e.geometry1, 'POINT(1 2)') FROM %s e", ContainsGeometries::class), + 'checks if literal is strictly to the left of geometry' => \sprintf("SELECT STRICTLY_LEFT('POINT(1 2)', e.geometry1) FROM %s e", ContainsGeometries::class), ];tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (1)
19-24: Mirror the StrictlyLeft test by adding a literal operand case.Parity across operator tests helps catch regressions in node mapping.
Apply:
protected function getExpectedSqlStatements(): array { return [ 'checks if geometry is strictly above' => 'SELECT (c0_.geometry1 |>> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'checks if geometry is strictly above literal' => "SELECT (c0_.geometry1 |>> 'POINT(0 -2)') AS sclr_0 FROM ContainsGeometries c0_", ]; }protected function getDqlStatements(): array { return [ 'checks if geometry is strictly above' => \sprintf('SELECT STRICTLY_ABOVE(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'checks if geometry is strictly above literal' => \sprintf("SELECT STRICTLY_ABOVE(e.geometry1, 'POINT(0 -2)') FROM %s e", ContainsGeometries::class), ]; }tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (2)
19-25: Add the symmetric literal-on-the-left case.Covers both operand orders and guards against asymmetric node handling.
protected function getExpectedSqlStatements(): array { return [ 'calculates distance between geometries' => 'SELECT (c0_.geometry1 <-> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', 'calculates distance between geometry and literal' => "SELECT (c0_.geometry1 <-> 'POINT(1 2)') AS sclr_0 FROM ContainsGeometries c0_", + 'calculates distance between literal and geometry' => "SELECT ('POINT(1 2)' <-> c0_.geometry1) AS sclr_0 FROM ContainsGeometries c0_", ]; }protected function getDqlStatements(): array { return [ 'calculates distance between geometries' => \sprintf('SELECT GEOMETRY_DISTANCE(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), 'calculates distance between geometry and literal' => \sprintf("SELECT GEOMETRY_DISTANCE(e.geometry1, 'POINT(1 2)') FROM %s e", ContainsGeometries::class), + 'calculates distance between literal and geometry' => \sprintf("SELECT GEOMETRY_DISTANCE('POINT(1 2)', e.geometry1) FROM %s e", ContainsGeometries::class), ]; }
22-24: Ensure correct documentation of<->operator semantics. The PostGIS<->operator returns adouble precision2D distance between two geometries. In PostGIS 2.0.0 (PostgreSQL 9.1+) it provided a “weak KNN” approximate distance based on bounding‐box centroids, whereas since PostGIS 2.2.0 (PostgreSQL 9.5+) it performs true KNN, yielding the exact Euclidean distance between geometries (and spherical distances for geographies) (postgis.net). Please update or add comments in your integration tests (lines 22–24 and 30–31 oftests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php) to clearly state this behavior for test consumers.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (1)
19-31: Add a column-based case to exercise hydration and column operands.Right now all assertions use literal WKT. Add at least one test that operates on entity columns (e.g., two XYM LINESTRING M rows) to ensure the operator works when Doctrine renders column refs.
Apply at the end of the class:
+ #[Test] + public function trajectory_distance_with_entity_columns(): void + { + // Precondition: ensure there is at least one row with XYM (measured) lines in the fixture table (see note below). + $dql = 'SELECT TRAJECTORY_DISTANCE(g.geometry1, g.geometry2) as distance + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 4'; + + $result = $this->executeDqlQuery($dql); + $this->assertIsNumeric($result[0]["distance"]); + $this->assertGreaterThanOrEqual(0, $result[0]["distance"]); + }Note: To support this, extend your fixture seeding to insert a row with XYM, e.g.:
// tests/Integration/.../SpatialOperatorTestCase.php -> insertTestDataForSpatialFixture() $testData[] = [ 'geometry1' => 'LINESTRING M(0 0 0, 5 5 10)', 'geometry2' => 'LINESTRING M(1 1 0, 6 6 10)', 'geography1' => 'SRID=4326;POINT(-9.1393 38.7223)', 'geography2' => 'SRID=4326;POINT(-0.1276 51.5074)', ];tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (1)
19-28: Strengthen assertions to expected truth valuesGiven the seeded fixture data, these cases are deterministic. Prefer asserting true/false to catch regressions, not just type.
For example:
- $this->assertIsBool($result[0]['result']); + $this->assertFalse($result[0]['result']); // id=2 polygons overlap in Y, so not strictly belowand
- $this->assertIsBool($result[0]['result']); + $this->assertTrue($result[0]['result']); // id=3 linestring1 is strictly below linestring2If you prefer to keep smoke tests, consider adding separate assertion-focused tests.
Also applies to: 41-50
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (1)
22-26: Minor: keep DQL minimal to reduce noise (optional)The WHERE g.id = 1 clause is only there to make the FROM valid. Consider a tiny helper in SpatialOperatorTestCase that returns a standard FROM/WHERE stub to avoid repeating the fixture reference and magic id.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (2)
55-55: Use delta-based float assertions for stabilityEven with planar geometry, slight differences can occur across PostGIS versions. Use a small delta instead of strict equality.
Apply this diff:
- $this->assertEquals(5.0, $result[0]['distance'], 'Horizontal distance should be 5'); + $this->assertEqualsWithDelta(5.0, $result[0]['distance'], 1e-6, 'Horizontal distance should be 5');- $this->assertEquals(3.0, $result[0]['distance'], 'Vertical distance should be 3'); + $this->assertEqualsWithDelta(3.0, $result[0]['distance'], 1e-6, 'Vertical distance should be 3');Also applies to: 67-67
73-81: Optional: assert a tighter range for geography distanceIf you want stronger guarantees, consider asserting an expected range for Lisbon–London (e.g., 1,500–2,500 km) instead of only “> 1,000,000 m”. This keeps the test informative without being brittle.
Example:
- $this->assertGreaterThan(1000000, $result[0]['distance']); // Lisbon to London is > 1M meters + $this->assertGreaterThan(1_000_000, $result[0]['distance']); // sanity check + $this->assertLessThan(3_000_000, $result[0]['distance']); // guard against unit/degree regressions
📜 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 (23)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (23)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (3)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
StrictlyLeft(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (2)
StrictlyLeftTest(10-66)getStringFunctions(12-17)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (3)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
StrictlyAbove(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (2)
StrictlyAboveTest(10-51)getStringFunctions(12-17)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
BoundingBoxDistance(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php (2)
BoundingBoxDistanceTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (3)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
StrictlyRight(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (2)
StrictlyRightTest(10-51)getStringFunctions(12-17)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php (2)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
BoundingBoxDistance(21-29)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
customizeFunction(22-27)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
customizeFunction(22-27)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (2)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
GeometryDistance(21-29)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php (1)
TrajectoryDistance(21-29)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
TrajectoryDistanceTest(10-54)getStringFunctions(12-17)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php (1)
TrajectoryDistance(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
TrajectoryDistanceTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
customizeFunction(22-27)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
customizeFunction(23-28)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
StrictlyAbove(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (2)
StrictlyAboveTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (2)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
StrictlyBelow(20-28)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (7)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
customizeFunction(22-27)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
customizeFunction(22-27)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
StrictlyRight(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
StrictlyLeft(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
GeometryDistance(21-29)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
StrictlyBelow(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (2)
StrictlyBelowTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/WktSpatialData.php (1)
WktSpatialData(22-109)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (1)
customizeFunction(23-28)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (2)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)tests/Integration/MartinGeorgiev/TestCase.php (2)
createTestSchema(159-179)dropTestTableIfItExists(239-246)
🪛 GitHub Actions: Integration Tests
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php
[error] 51-51: Step: composer run-integration-tests. strictly_left_returns_false_when_left_geometry_is_to_the_right: Ambiguous function: operator is not unique: unknown << unknown. Could not choose a best candidate operator; you might need to add explicit type casts.
[error] 27-27: Step: composer run-integration-tests. strictly_left_returns_true_when_left_geometry_is_strictly_to_the_left: Ambiguous function: operator is not unique: unknown << unknown. Could not choose a best candidate operator; you might need to add explicit type casts.
[error] 39-39: Step: composer run-integration-tests. strictly_left_returns_false_when_geometries_overlap: Ambiguous function: operator is not unique: unknown << unknown. Could not choose a best candidate operator; you might need to add explicit type casts.
[error] 63-63: Step: composer run-integration-tests. strictly_left_returns_false_with_identical_geometries: Ambiguous function: operator is not unique: unknown << unknown. Could not choose a best candidate operator; you might need to add explicit type casts.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php
[error] 66-66: Step: composer run-integration-tests. Ambiguous function: operator is not unique: unknown <-> unknown. Could not choose a best candidate operator; you might need to add explicit type casts.
[error] 66-66: Step: composer run-integration-tests. Ambiguous function: operator is not unique: unknown <-> unknown. Could not choose a best candidate operator; you might need to add explicit type casts.
⏰ 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). (12)
- 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.2 + 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 latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + 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.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
🔇 Additional comments (31)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
11-16: Solid test base setup; creates predictable spatial fixtureThe setUp flow is clear and keeps tests hermetic by recreating the schema and data per test run.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
12-17: Function binding looks correct for the CPA trajectory operatorMapping 'TRAJECTORY_DISTANCE' => TrajectoryDistance::class is consistent with using the |=| operator (closest point of approach). (postgis.net, access.crunchydata.com)
19-24: Expected SQL is spot onThe generated SQL uses (c0_.geometry1 |=| c0_.geometry2), matching PostGIS operator semantics.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
20-27: Correct SQL prototype and node mapping for “strictly below”The <<| operator and two StringPrimary operands match PostGIS bounding-box semantics for “strictly below.” (postgis.net)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (2)
12-17: Function binding matches implementationSTRICTLY_RIGHT => StrictlyRight::class aligns with the operator prototype (%s >> %s).
19-24: Expected SQL assertion is accurateUses (c0_.geometry1 >> c0_.geometry2), which is the PostGIS “strictly to the right” bbox operator. (postgis.net)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
20-27: Implementation is consistent and correctPrototype (%s >> %s) and StringPrimary mappings align with PostGIS “strictly to the right” operator. (postgis.net)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php (1)
23-28: Operator mapping and node configuration look correctPrototype '(%s |=| %s)' and two StringPrimary operands align with the intended PostGIS trajectory distance operator. Consistent with the existing pattern in the library.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php (2)
12-17: Function mapping is consistent and clearRegisters BOUNDING_BOX_DISTANCE → BoundingBoxDistance::class following the library’s conventions.
19-24: SQL expectation matches the intended operator and aliasing schemeThe expected SQL fragment uses (<#>) and aligns with the library’s SqlWalker output and table alias conventions.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
22-27: LGTM: operator prototype and nodes are correctPrototype '(%s << %s)' with two StringPrimary operands matches the “strictly left” bbox operator.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistance.php (2)
23-28: Correct prototype (<#>) and two-operand mappingMatches the PostGIS 2D bbox distance operator and follows the house style.
13-14: Double-check the @see anchor points to the specific operatorThe general Operators_Distance section is fine, but if PostGIS provides a direct anchor for <#>, consider linking to it to spare readers a scroll.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAbove.php (1)
22-27: LGTM: correct operator mapping and node parsers.Operator prototype '(%s |>> %s)' and two StringPrimary operands match the PostGIS |>> semantics and are consistent with the sibling operators in this PR. (postgis.net)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (1)
12-17: LGTM: proper string function mapping.'STRICTLY_BELOW' → StrictlyBelow::class registration matches the implementation.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
23-28: LGTM: correct operator prototype and node mappings.'(%s <-> %s)' with two StringPrimary operands is consistent with BaseFunction usage and with PostGIS operator expectations. (postgis.net)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
10-23: LGTM overall for the fixture shape.Types line up with your DBAL
WktSpatialDatavalue object and the simple geometry/geography columns suit the operator tests well.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (2)
12-17: LGTM: correct DQL-to-operator wiring.
STRICTLY_LEFTis mapped to the expected class; consistent with other operator tests.
21-24: No change needed: PostGIS implicitly casts WKT literals to geometryPostGIS defines an automatic cast from text to geometry, so when you write
SELECT (c0_.geometry1 << 'POINT(1 2)') …the
'POINT(1 2)'literal is implicitly converted to a geometry before the<<operator is applied (stackoverflow.com). This behavior is documented via the PostgreSQL\dCcast list and in the PostGIS reference (postgis.net).If you’d like to be extra explicit (and avoid any chance of ambiguous overload resolution), you can opt to cast or wrap with the constructor—both will work identically in practice:
'POINT(1 2)'::geometryST_GeomFromText('POINT(1 2)')Since the implicit cast is always available once PostGIS is installed, the existing tests in StrictlyLeftTest.php (lines 21–24, 29–32) are already correct and need no modification.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php (2)
12-17: LGTM: operator mapping and shape look right.
STRICTLY_ABOVE→|>>is aligned with PostGIS operator semantics.
22-22: I’m not yet certain which “|>>” operator you’re using (the geometry “rightmost point of bounding box” operator vs. a custom AST function), nor which PostGIS/PostgreSQL version you target. Could you clarify:• Exactly which PostGIS “|>>” operator you mean (e.g. the bounding‐box operator defined in the PostGIS docs)?
• Your target PostGIS version (and PostgreSQL version, if relevant)?With that information, I can point to the precise casting requirements and authoritative reference.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (1)
12-17: LGTM: function wiring and expectations are consistent.
GEOMETRY_DISTANCE→<->with twoStringPrimaryoperands is consistent with your AST function pattern.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
19-31: Great coverage of literals and result properties.Three scenarios validate numeric output, identity = 0, and non-negativity on more complex inputs. Good smoke tests for the operator.
22-23: Operator|=|Availability and Operand Requirements
- The
|=|trajectory‐distance operator was introduced in PostGIS 2.2 and is available in all subsequent releases (2.3, 2.5, 3.x). (postgis.net)- It appears in the official PostGIS 3.4 reference under “Distance Operators” in section 7.10.2. (postgis.net)
- Operand requirements: both operands must be trajectory geometries—that is, linear geometries carrying an M (measure) value on each vertex (e.g.
LINESTRINGMorLINESTRINGZMwith monotonically increasing M). (postgis.net)Please ensure your CI/PostGIS extension is at least version 2.2 (or guard/skip this test on older versions) so that the
|=|operator is supported.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (1)
12-17: DQL function registration looks goodMapping STRICTLY_BELOW to its implementation is correct and matches the function prototype in StrictlyBelow.php.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (3)
12-17: DQL function registration looks goodSTRICTLY_LEFT is correctly registered and matches the SQL operator prototype (%s << %s).
20-29: Nice set of positive/negative casesThe assertions reflect true/false scenarios and read well. Once the literal typing is fixed, these should stabilize.
Also applies to: 32-41, 44-53, 56-65
27-27: Follow-up: Re-run Integration Tests After Literal-Typing FixPlease ensure you’ve installed the project’s development dependencies (e.g., via
composer install) and then re-run the integration suite to confirm that the “operator is not unique” errors have been resolved. You can do this either by:
- Running the PHPUnit binary directly:
vendor/bin/phpunit --configuration=./ci/phpunit/config-integration.xml- Or via Composer script:
composer run-integration-testsOnce you’ve applied the literal-typing fixes, verify that these failures no longer occur in tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php at lines 27, 39, 51, and 63.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (3)
12-17: DQL function registration looks goodGEOMETRY_DISTANCE is correctly mapped to the <-> operator.
27-32: Good coverage of mixed casesThe sqrt(2) and identical-geometry cases are clear and useful sanity checks. With the literal-typing fixes, these should be stable across engines.
Also applies to: 42-44
66-66: Re-run integration tests after installing dependenciesIt looks like the integration suite can’t find the PHPUnit binary (vendor/bin/phpunit) or any test dependencies in this environment. Before verifying your typing changes have fixed the CI failures, please make sure to:
- Run
composer installto pull in all dependencies (including PHPUnit).- Confirm that
vendor/bin/phpunitnow exists.- Re-run the integration suite with:
composer run-integration-testsOnce that passes, the two failing tests in
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php(around line 66) should be resolved.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php
Show resolved
Hide resolved
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php
Outdated
Show resolved
Hide resolved
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php
Outdated
Show resolved
Hide resolved
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (1)
10-17: Add a guard test using a typed literal to prevent “operator is not unique: unknown <<| unknown”We previously hit ambiguity when using plain string WKTs. Add one test that uses Doctrine’s FUNCTION() pass-through to construct a PostGIS geometry from WKT so the operator overload resolves deterministically.
Example to append to this class:
#[Test] public function strictly_below_with_typed_literal_right_operand(): void { // g.id = 1 has geometry1 = POINT(0 0); compare against a literal POINT(0 5) $dql = "SELECT STRICTLY_BELOW( g.geometry1, FUNCTION('ST_GeomFromText', 'POINT(0 5)') ) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = :id"; $result = $this->executeDqlQuery($dql, ['id' => 1]); $this->assertIsBool($result[0]['result']); $this->assertTrue($result[0]['result']); }Alternative: bind a parameter of Doctrine “geometry” type (e.g., WktSpatialData) instead of inlining the literal.
🧹 Nitpick comments (4)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (4)
19-29: Strengthen assertions and bind the ID parameter
- Add assertIsBool() to lock the return type and catch platform mapping issues early.
- Use a bound parameter for the entity id to be consistent with the test harness and avoid inlined literals.
Apply this diff:
- $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 2'; + $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry2) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = :id'; - $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql, ['id' => 2]); + $this->assertIsBool($result[0]['result']); $this->assertFalse($result[0]['result'], 'Overlapping polygons should not be strictly below each other');
31-41: Same refinements here: parameter binding and explicit bool assertionKeeps tests consistent and slightly more robust across drivers/platforms.
- $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry2) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = :id'; - $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql, ['id' => 1]); + $this->assertIsBool($result[0]['result']); $this->assertTrue($result[0]['result'], 'POINT(0 0) should be strictly below POINT(1 1)');
43-53: Apply the same improvements for the linestrings caseBound parameter and explicit boolean type check.
- $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 3'; + $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry2) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = :id'; - $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql, ['id' => 3]); + $this->assertIsBool($result[0]['result']); $this->assertTrue($result[0]['result'], 'Lower linestring should be strictly below higher linestring');
55-65: Same: bind parameter and assert boolean typeMirrors the pattern used in the other tests for consistency.
- $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry1) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = 'SELECT STRICTLY_BELOW(g.geometry1, g.geometry1) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = :id'; - $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql, ['id' => 1]); + $this->assertIsBool($result[0]['result']); $this->assertFalse($result[0]['result'], 'Identical geometries should not be strictly below each other');
📜 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)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyAboveTest.php
🧰 Additional context used
🧠 Learnings (4)
📚 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/ORM/Query/AST/Functions/StrictlyBelowTest.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/ORM/Query/AST/Functions/StrictlyBelowTest.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/ORM/Query/AST/Functions/StrictlyBelowTest.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:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php
🧬 Code graph analysis (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelow.php (1)
StrictlyBelow(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (2)
StrictlyBelowTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
🔇 Additional comments (2)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyBelowTest.php (2)
12-17: Function map looks correctMapping STRICTLY_BELOW => StrictlyBelow::class aligns with the BaseFunction prototype '(%s <<| %s)' and similar tests in the suite.
7-9: PHPUnit attribute support verified ✅
- composer.json declares phpunit/phpunit at “^10.5.53”, which includes attribute support for #[Test]
- Numerous tests (including StrictlyBelowTest) import and use
PHPUnit\Framework\Attributes\Testwithout errorsNo further action needed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (1)
1-56: Add an integration test for typed WKT literals to prevent “operator is not unique: unknown <-> unknown” regressionsIntegration coverage for the ST_GeomFromText pass-through ensures we don’t reintroduce the ambiguous-operator issue when literals are involved.
Proposed test method (place below the existing tests):
#[Test] public function geometry_distance_with_typed_wkt_literals(): void { // Using FUNCTION('ST_GeomFromText', ...) ensures operands are typed to geometry $dql = \sprintf( "SELECT GEOMETRY_DISTANCE( FUNCTION('ST_GeomFromText', :wkt1), FUNCTION('ST_GeomFromText', :wkt2) ) as distance FROM %s g WHERE g.id = 1", SpatialFixture::class ); $result = $this->executeDqlQuery($dql, [ 'wkt1' => 'POINT(0 0)', 'wkt2' => 'POINT(5 0)', ]); $this->assertCount(1, $result); $this->assertIsNumeric($result[0]['distance']); $this->assertEqualsWithDelta(5.0, (float) $result[0]['distance'], 0.0001, 'Distance along x-axis should be 5'); }I can push this as a follow-up commit if you’d like.
🧹 Nitpick comments (4)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (4)
26-31: Strengthen assertions and guard against empty result setsTwo small robustness tweaks:
- Assert the query returned exactly one row before indexing.
- Use sqrt(2) for precision and cast the DB scalar to float to avoid string-vs-float pitfalls; tighten the delta.
Apply this diff:
$result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertIsNumeric($result[0]['distance']); $this->assertGreaterThan(0, $result[0]['distance']); - // Distance between (0,0) and (1,1) should be sqrt(2) ≈ 1.414 - $this->assertEqualsWithDelta(1.414, $result[0]['distance'], 0.01, 'Distance between (0,0) and (1,1)'); + // Distance between (0,0) and (1,1) should be sqrt(2) ≈ 1.41421356 + $this->assertEqualsWithDelta(\sqrt(2), (float) $result[0]['distance'], 0.0001, 'Distance between (0,0) and (1,1)');
40-42: Make the zero-distance assertion type-safeAdd a numeric assertion for parity with the first test and compare as float with delta to avoid string-vs-float surprises from Doctrine hydration.
Apply this diff:
- $result = $this->executeDqlQuery($dql); - $this->assertEquals(0, $result[0]['distance'], 'Distance between identical geometries should be 0'); + $result = $this->executeDqlQuery($dql); + $this->assertIsNumeric($result[0]['distance']); + $this->assertEqualsWithDelta(0.0, (float) $result[0]['distance'], 0.0, 'Distance between identical geometries should be 0');
52-55: Tighten geography assertion with realistic bounds and explicit castCurrent threshold (> 1,000,000 m) is very loose. Casting once and asserting a reasonable range will better guard against regressions while remaining robust across spheroid/sphere differences.
Apply this diff:
- $result = $this->executeDqlQuery($dql); - $this->assertIsNumeric($result[0]['distance']); - $this->assertGreaterThan(1000000, $result[0]['distance']); // Lisbon to London is > 1M meters + $result = $this->executeDqlQuery($dql); + $this->assertIsNumeric($result[0]['distance']); + $distance = (float) $result[0]['distance']; + // Lisbon ↔ London great-circle distance ≈ 1.58e6 m (allow generous bounds) + $this->assertGreaterThan(1_400_000, $distance); + $this->assertLessThan(1_800_000, $distance);
7-8: Prefer ::class for the entity FQN to avoid brittle string literalsUsing the class constant reduces typo risk and eases refactors. Keeps the DQL concise and consistent with the unit tests.
Apply this diff:
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\GeometryDistance; use PHPUnit\Framework\Attributes\Test; +use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries as SpatialFixture; @@ - $dql = 'SELECT GEOMETRY_DISTANCE(g.geometry1, g.geometry2) as distance - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT GEOMETRY_DISTANCE(g.geometry1, g.geometry2) as distance FROM %s g WHERE g.id = 1', + SpatialFixture::class + ); @@ - $dql = 'SELECT GEOMETRY_DISTANCE(g.geometry1, g.geometry1) as distance - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT GEOMETRY_DISTANCE(g.geometry1, g.geometry1) as distance FROM %s g WHERE g.id = 1', + SpatialFixture::class + ); @@ - $dql = 'SELECT GEOMETRY_DISTANCE(g.geography1, g.geography2) as distance - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT GEOMETRY_DISTANCE(g.geography1, g.geography2) as distance FROM %s g WHERE g.id = 1', + SpatialFixture::class + );Also applies to: 22-24, 36-38, 48-50
📜 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 (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/ORM/Query/AST/Functions/GeometryDistanceTest.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/ORM/Query/AST/Functions/GeometryDistanceTest.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/ORM/Query/AST/Functions/GeometryDistanceTest.php
🧬 Code graph analysis (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistance.php (1)
GeometryDistance(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (2)
GeometryDistanceTest(10-34)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
⏰ 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: sloth
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.2
- 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 3.0
- GitHub Check: PHP 8.3 + 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 2.14 + 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 2.1
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php (1)
12-17: GEOMETRY_DISTANCE is correctly registered for DQLMapping looks correct and consistent with the rest of the suite.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.php (3)
19-24: Expected SQL matches operator semantics; add a symmetry checkThe expected SQL correctly emits the PostGIS n-D bounding box distance operator (<<#>>). Given how easy it is to confuse <<#>> with <<->>, consider adding a second assertion that swaps the operands to document symmetry and guard against regressions.
Apply this diff to add the symmetric case:
@@ protected function getExpectedSqlStatements(): array { return [ 'calculates n-dimensional bounding box distance' => 'SELECT (c0_.geometry1 <<#>> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'is symmetric with operands swapped' => 'SELECT (c0_.geometry2 <<#>> c0_.geometry1) AS sclr_0 FROM ContainsGeometries c0_', ]; }
26-31: Mirror the symmetry check in DQL statementsAdd the corresponding DQL for the swapped operands so keys align between DQL and SQL expectation maps.
Apply this diff:
@@ protected function getDqlStatements(): array { return [ 'calculates n-dimensional bounding box distance' => \sprintf('SELECT ND_BOUNDING_BOX_DISTANCE(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'is symmetric with operands swapped' => \sprintf('SELECT ND_BOUNDING_BOX_DISTANCE(e.geometry2, e.geometry1) FROM %s e', ContainsGeometries::class), ]; }
10-10: Optional: Consider an ORDER BY use-case testReal-world usage often orders by distance for KNN-like queries. If your base TestCase supports it (or in an integration test), add a case such as: SELECT e FROM ContainsGeometries e ORDER BY ND_BOUNDING_BOX_DISTANCE(e.geometry1, e.geometry2) ASC. This helps ensure the walker emits the operator correctly in ORDER BY contexts.
I can draft an additional test method or an integration test variant if your harness supports ORDER BY assertions.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php (1)
7-20: Tighten the docblock: note usage contexts and versioningTwo small tweaks will improve clarity:
- Call out that the operator is usable in ORDER BY (common KNN pattern).
- Verify the @SInCE tag (3.5) lines up with the planned library release that will contain this class.
Apply this diff to insert the ORDER BY example:
@@ * @example Using it in DQL: "SELECT ND_BOUNDING_BOX_DISTANCE(g1.geometry, g2.geometry) FROM Entity g1, Entity g2" + * @example Also usable in ORDER BY: "SELECT e FROM Entity e ORDER BY ND_BOUNDING_BOX_DISTANCE(e.geometry1, e.geometry2) ASC" * Returns numeric distance value.To confirm the @SInCE value and PostGIS support level, please verify:
- The target library version that will include this class.
- The minimum PostGIS version that supports the <<#>> n-D bounding box distance operator.
If helpful, I can add these constraints to the docblock once confirmed.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (1)
26-31: Optional: add a unit assertion for a literal WKT to validate StringPrimary parsing.To mirror the integration test and lock the SQL translation for string literals (not just path expressions), consider adding a second case using a WKT literal. This only validates SQL generation (no DB needed) and guards against regressions in the parser/walker.
Apply this diff to extend both lists:
@@ protected function getExpectedSqlStatements(): array { return [ 'calculates n-dimensional centroid distance' => 'SELECT (c0_.geometry1 <<->> c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'calculates distance to literal WKT' => "SELECT (c0_.geometry1 <<->> 'POINT(3 3)') AS sclr_0 FROM ContainsGeometries c0_", ]; } @@ protected function getDqlStatements(): array { return [ 'calculates n-dimensional centroid distance' => \sprintf('SELECT ND_CENTROID_DISTANCE(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'calculates distance to literal WKT' => \sprintf("SELECT ND_CENTROID_DISTANCE(e.geometry1, 'POINT(3 3)') FROM %s e", ContainsGeometries::class), ]; }tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (1)
10-53: Optional: add a commutativity check.Distance should be symmetric. A tiny extra test hardens behavior against accidental operand order bugs.
@@ class NDimensionalCentroidDistanceTest extends SpatialOperatorTestCase public function nd_centroid_distance_with_identical_geometries(): void { $dql = 'SELECT ND_CENTROID_DISTANCE(g.geometry1, g.geometry1) as distance FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 1'; $result = $this->executeDqlQuery($dql); $this->assertEquals(0, $result[0]['distance']); } + + #[Test] + public function nd_centroid_distance_is_commutative(): void + { + $dqlA = 'SELECT ND_CENTROID_DISTANCE(g.geometry1, g.geometry2) as d + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 1'; + $dqlB = 'SELECT ND_CENTROID_DISTANCE(g.geometry2, g.geometry1) as d + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 1'; + + $a = $this->executeDqlQuery($dqlA)[0]['d']; + $b = $this->executeDqlQuery($dqlB)[0]['d']; + $this->assertSame($a, $b); + }src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (1)
7-20: Doc nit: clarify wording and cross-reference exact PostGIS operator name.Tiny doc polish to match PostGIS terminology and add an explicit operator mention.
- * Implementation of PostGIS n-D centroid distance operator (using <<->>). + * Implementation of the PostGIS n-D centroid distance operator "<<->>". @@ - * Returns the n-D distance between the centroids of A and B bounding boxes. + * Returns the n-D distance between the centroids of the bounding boxes of A and B. @@ - * @see https://postgis.net/docs/reference.html#Operators_Distance + * @see https://postgis.net/docs/reference.html#Operators_Distancesrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (2)
7-20: Tighten the docblock: clarify 2D behavior and DQL usage guidance.The '&&&' operator is n‑dimensional but also works for 2D inputs; noting this avoids confusion. Also, instead of a hard “must be used with = TRUE/= FALSE”, consider softer guidance that some DQL contexts (esp. WHERE) may require explicit comparison depending on platform/version.
Proposed doc tweak:
/** - * Implementation of PostGIS n-dimensional bounding box intersects operator (using &&&). + * Implementation of PostGIS n-dimensional bounding box intersects operator (&&&). * - * Returns TRUE if A's n-D bounding box intersects B's n-D bounding box. - * This operator works with 3D and higher dimensional geometries. + * Returns TRUE if A's n‑D bounding box intersects B's n‑D bounding box. + * Works for 2D geometries as well; for 3D+ it considers Z/M where present. * * @see https://postgis.net/docs/reference.html#Operators_Geometry * @since 3.5 * * @author Martin Georgiev <martin.georgiev@gmail.com> * - * @example Using it in DQL with boolean comparison: "WHERE ND_OVERLAPS(g1.geometry, g2.geometry) = TRUE" - * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. + * @example WHERE ND_OVERLAPS(g1.geometry, g2.geometry) = TRUE + * Returns boolean. In some DQL contexts you may need explicit comparison to TRUE/FALSE. */
21-21: Consider marking the class final.These DQL function nodes are not designed for extension; marking final prevents accidental inheritance and matches how many similar nodes are shipped in libraries.
-class NDimensionalOverlaps extends BaseFunction +final class NDimensionalOverlaps extends BaseFunctiontests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (2)
19-28: Strengthen assertions with type checks.Add an explicit type assertion to ensure DB/driver returns a boolean, then assert truthiness.
- $result = $this->executeDqlQuery($dql); - $this->assertTrue($result[0]['result'], 'Overlapping polygons should return true for n-dimensional overlaps'); + $result = $this->executeDqlQuery($dql); + $this->assertIsBool($result[0]['result']); + $this->assertTrue($result[0]['result'], 'Overlapping polygons should return true for n-dimensional overlaps');
30-39: Add ND-specific 3D case and a non-overlap negative case.To demonstrate the n‑dimensional nature: two points that coincide in XY but differ in Z should yield FALSE with &&& (would be TRUE with &&). Also add a 2D non‑overlap to guard regressions. These don’t require changing the fixture data (we can evaluate with a literal vs literal in the SELECT while still selecting from the fixture entity).
#[Test] public function nd_overlaps_with_identical_geometries(): void { $dql = 'SELECT ND_OVERLAPS(g.geometry1, g.geometry1) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 1'; $result = $this->executeDqlQuery($dql); $this->assertTrue($result[0]['result']); } + + #[Test] + public function nd_overlaps_3d_points_same_xy_different_z_is_false(): void + { + // Demonstrates true n-D behavior: same XY, different Z → boxes don't intersect in 3D + $dql = "SELECT ND_OVERLAPS('POINT Z(0 0 0)', 'POINT Z(0 0 1)') as result + FROM Fixtures\\MartinGeorgiev\\Doctrine\\Entity\\ContainsGeometries g + WHERE g.id = 1"; + + $result = $this->executeDqlQuery($dql); + $this->assertIsBool($result[0]['result']); + $this->assertFalse($result[0]['result']); + } + + #[Test] + public function nd_overlaps_with_non_overlapping_2d_lines_is_false(): void + { + $dql = 'SELECT ND_OVERLAPS(g.geometry1, g.geometry2) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 3'; // linestrings with disjoint boxes in the seeded data + + $result = $this->executeDqlQuery($dql); + $this->assertIsBool($result[0]['result']); + $this->assertFalse($result[0]['result']); + }tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (1)
19-33: Consider adding a literal-vs-literal 3D translation case.This captures the 3D semantics at the SQL generator level without needing extra fixtures.
protected function getExpectedSqlStatements(): array { return [ 'checks if geometries overlap in n-dimensions' => 'SELECT (c0_.geometry1 &&& c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', 'checks if geometry overlaps literal in n-dimensions' => "SELECT (c0_.geometry1 &&& 'POINT Z(1 2 3)') AS sclr_0 FROM ContainsGeometries c0_", + 'checks 3d literals do not overlap when only z differs' => "SELECT ('POINT Z(0 0 0)' &&& 'POINT Z(0 0 1)') AS sclr_0 FROM ContainsGeometries c0_", ]; } protected function getDqlStatements(): array { return [ 'checks if geometries overlap in n-dimensions' => \sprintf('SELECT ND_OVERLAPS(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), 'checks if geometry overlaps literal in n-dimensions' => \sprintf("SELECT ND_OVERLAPS(e.geometry1, 'POINT Z(1 2 3)') FROM %s e", ContainsGeometries::class), + 'checks 3d literals do not overlap when only z differs' => \sprintf("SELECT ND_OVERLAPS('POINT Z(0 0 0)', 'POINT Z(0 0 1)') FROM %s e", ContainsGeometries::class), ]; }
📜 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 (9)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php
🧰 Additional context used
🧠 Learnings (4)
📚 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/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.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/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.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/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.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:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php
🧬 Code graph analysis (8)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (1)
NDimensionalCentroidDistance(21-29)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (2)
NDimensionalCentroidDistanceTest(10-53)getStringFunctions(12-17)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (1)
NDimensionalOverlaps(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (2)
NDimensionalOverlapsTest(10-34)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (1)
customizeFunction(23-28)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (1)
customizeFunction(23-28)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (1)
customizeFunction(23-28)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (1)
NDimensionalOverlaps(21-29)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (2)
NDimensionalOverlapsTest(10-40)getStringFunctions(12-17)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php (1)
NDimensionalBoundingBoxDistance(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (3)
getStringFunctions(12-17)getExpectedSqlStatements(19-24)getDqlStatements(26-31)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (1)
NDimensionalCentroidDistance(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (2)
NDimensionalCentroidDistanceTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
⏰ 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.3 + 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 3.0 + Doctrine Lexer 3.0
- 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.1 + Doctrine ORM 2.14 + Doctrine Lexer 1.2
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- 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.3 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: sloth
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (14)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistanceTest.php (2)
7-9: Imports and namespace usage look correctUsing the fixture entity and the class under test is spot on. No need to import TestCase explicitly in this namespace, per the project’s testing convention.
12-17: Correct DQL function registrationThe mapping of ND_BOUNDING_BOX_DISTANCE to NDimensionalBoundingBoxDistance aligns with the production class and other ND operator tests.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalBoundingBoxDistance.php (1)
23-28: Implementation aligns with existing ND operator patternFunction prototype '(%s <<#>> %s)' and two StringPrimary operands are consistent with the project’s BaseFunction pattern and sibling classes (e.g., ND centroid distance, overlaps).
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (2)
12-17: LGTM: correct DQL function mapping for ND_CENTROID_DISTANCE.Alias → class mapping looks consistent with the library’s pattern. Good use of the custom TestCase base (per prior learnings).
19-24: Expected SQL matches operator form and parentheses.The prototype renders as "(%s <<->> %s)" and the SQL expectation reflects that exactly. No issues.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php (4)
12-17: LGTM: function registration in integration test harness.Registration is minimal and consistent with other spatial-operator tests.
19-29: Sensible assertions for geometry-to-geometry case.For POINT vs POINT this should be sqrt(2); asserting > 0 keeps it resilient across engines/rounding. Good.
31-41: Confirm cross-version behavior for WKT literal without explicit cast.Using a bare string literal ('POINT(3 3)') relies on PostgreSQL/PostGIS to infer geometry from an unknown literal in an operator context. This typically works where an implicit cast exists, but can be version/extension dependent and risk flakiness.
If CI covers a matrix of PostgreSQL/PostGIS versions, please confirm it passes there. If you hit issues, two robust alternatives:
- Use a DQL function that wraps ST_GeomFromText (if available in this library) and register it in getStringFunctions() for this test.
- Or replace the literal with a persisted fixture geometry (mirroring Line 22’s approach).
I can propose a concrete patch once we confirm which casting helpers are available in this repo.
43-52: Nice: identical geometry returns 0.Covers the zero-distance edge case succinctly.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistance.php (2)
23-28: LGTM: correct operator prototype and node mappings.
- Prototype includes parentheses to avoid precedence surprises.
- Two StringPrimary operands are consistent with sibling ND operator implementations.
No functional concerns spotted.
14-15: Verify @SInCE annotation aligns with release planning.Ensure "since 3.5" matches the intended library version this feature will ship in, for accurate API docs.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlaps.php (1)
23-28: Operator mapping and parsing look correct.Prototype '(%s &&& %s)' and two StringPrimary operands align with how sibling ND_* operators are implemented in this codebase. SQL dispatching through BaseFunction will yield the expected "(arg1 &&& arg2)".
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (1)
7-11: PHPUnit 10+ attribute support confirmedAll tests use PHP 8 attributes (#[Test]) and our toolchain already targets PHPUnit 10:
- composer.json specifies
"phpunit/phpunit": "^10.5.53", which natively supports #[Test] attributes.- A full scan of
tests/shows consistent use of #[Test] and other PHPUnit attributes.- No phpunit.xml(.dist) was found—PHPUnit’s default configuration will pick up attributes without requiring a bridge.
No further changes needed here.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php (1)
12-33: Unit expectations for SQL/DQL translation look good.
- Mapping ND_OVERLAPS to the node class is correct.
- Expected SQL matches "(c0_.geometry1 &&& c0_.geometry2)" and a geometry‑literal case.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (13)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (4)
19-28: Solid positive-case scenario; consider asserting row count as a guardrail.The polygon-vs-polygon case at id = 2 should evaluate to true via “overlaps” semantics. As a small hardening, you can assert the result set size before inspecting the scalar to avoid accidental multi-row returns from future fixture changes.
Example:
- Add: $this->assertCount(1, $result);
41-50: This test duplicates the first scenario; repurpose it to cover geographies.The current query is identical to overlaps_below_with_geometries(). To broaden coverage, switch to geography columns; the Lisbon vs London polygons at id = 2 should satisfy “below” without overlap, exercising the operator’s alternative path on the GEOGRAPHY type.
Apply this diff:
- #[Test] - public function overlaps_below_with_polygons(): void + #[Test] + public function overlaps_below_with_geographies(): void { - $dql = 'SELECT OVERLAPS_BELOW(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 2'; + $dql = 'SELECT OVERLAPS_BELOW(g.geography1, g.geography2) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 2'; $result = $this->executeDqlQuery($dql); - $this->assertTrue($result[0]["result"], "Overlapping or below geometries should return true"); + $this->assertTrue((bool) $result[0]['result'], 'Overlapping or below geographies should return true'); }
51-51: Add a negative-case asserting false when the first geometry is above the second.You currently test “overlaps” and “below.” Add a complementary “above” case to assert false to better pin operator semantics and guard against regressions.
Apply this diff just before the closing brace:
+ #[Test] + public function overlaps_below_returns_false_when_above(): void + { + $dql = "SELECT OVERLAPS_BELOW(g.geometry1, 'POINT(0 -2)') as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 1"; + + $result = $this->executeDqlQuery($dql); + $this->assertFalse((bool) $result[0]['result'], 'Non-overlapping and above geometries should return false'); + }
30-39: No existing DQL “GeomFromText” helper – consider adding one or keep literalI searched the codebase for any DQL function that wraps ST_GeomFromText (e.g.
GeomFromTextorST_GeomFromText) and found only raw SQL calls in the test fixture setup – no library-level DQL helper exists insrc(only intests/Integration/.../SpatialOperatorTestCase.php)【run_scripts】.
- If you want to make the WKT-to-geometry conversion explicit in DQL, you’ll need to implement a new DQL function (e.g.
GeomFromText) that maps toST_GeomFromTextand register it with Doctrine.- Otherwise, relying on the literal
'POINT(0 2)'and implicit casting is acceptable for now.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (4)
19-28: Clarify the assertion message to reflect operator semantics (above-with-horizontal-overlap).The test logic is correct: for id = 2, geometry1 is below geometry2, so OVERLAPS_ABOVE(geometry1, geometry2) should be false. The current message is ambiguous. Recommend tightening the message to avoid implying overlap alone is sufficient.
Apply this diff:
- $this->assertFalse($result[0]['result'], 'Overlapping polygons may not be considered above each other'); + $this->assertFalse( + $result[0]['result'], + 'Expected false: geometry1 is not above geometry2 even if their bounding boxes overlap.' + );
44-49: Align assertion message with operator intent.The expectation is right; message should emphasize “above with horizontal overlap,” not “overlapping or above.”
Apply this diff:
- $this->assertTrue($result[0]['result'], 'Overlapping or above geometries should return true'); + $this->assertTrue( + $result[0]['result'], + 'Expected true: geometry2 is above geometry1 and their x-projections overlap.' + );
41-50: Add a non-overlap sanity check (id = 3, lines) to pin down the “overlaps-above” behavior.Row 3 contains two disjoint lines whose x-intervals do not overlap; even though one is vertically above the other, the operator should return false. Adding this guards against regressions in bbox logic.
Example test to add (outside this hunk):
#[Test] public function overlaps_above_with_disjoint_lines_should_be_false(): void { $dql = 'SELECT OVERLAPS_ABOVE(g.geometry2, g.geometry1) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 3'; $result = $this->executeDqlQuery($dql); $this->assertFalse( $result[0]['result'], 'Expected false: no horizontal overlap in x-projections even though geometry2 is vertically above.' ); }I can add the above test (and a geography variant) in this PR if you’d like.
33-38: Add a boolean‐type assertion but keep the existing DQL quotingTo keep the integration tests consistent (they all use double-quoted, multi-line strings for DQL), let’s skip converting to a nowdoc here. However, it’s still worth asserting the returned value’s type before checking its truthiness:
• File tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php
Update around the assertion to:$result = $this->executeDqlQuery($dql); $this->assertIsBool($result[0]['result']); $this->assertTrue( $result[0]['result'], 'Expected true: geometry1 is above the literal point and their x-projections overlap.' );This adds a quick guard against platform-specific boolean serialization without diverging from the suite’s existing DQL string style.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (5)
19-28: Tighten assertions and clarify the expectation wording for the points case.
- Add a quick sanity check that exactly one row is returned and that the value is a boolean.
- Since POINT(0 0) and POINT(1 1) don’t overlap, the reason this is TRUE is the “left of” part of
&<. The doc definition: A’s bbox overlaps OR is to the left of B’s.Apply this diff:
$result = $this->executeDqlQuery($dql); - $this->assertTrue($result[0]['result'], 'POINT(0 0) should overlap or be to the left of POINT(1 1)'); + $this->assertCount(1, $result); + $this->assertIsBool($result[0]['result']); + $this->assertTrue($result[0]['result'], 'POINT(0 0) should be to the left of POINT(1 1)');Why: For these inputs, “left of” is the active condition; the points don’t overlap but A is left of B, so
&<is TRUE. (postgis.net)
30-39: Mirror the extra assertions and keep the message crisp for the reversed points case.Add the same sanity checks as above; expectation is FALSE since POINT(1 1) is right of POINT(0 0).
$result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); + $this->assertIsBool($result[0]['result']); $this->assertFalse($result[0]['result'], 'POINT(1 1) should NOT be to the left of POINT(0 0)');Rationale:
&<requires “overlaps or left of”; neither holds when the first point is to the right. (postgis.net)
41-50: Good polygon scenario; consider wording to emphasize bbox semantics and add sanity checks.The result should be TRUE because the first bbox overlaps the second (squares [0..2]x[0..2] and [1..3]x[1..3] intersect). Add the same array-size and type assertions and clarify the message to mention “bounding box”.
$result = $this->executeDqlQuery($dql); - $this->assertTrue($result[0]['result'], 'First polygon should overlap or be to the left of second polygon'); + $this->assertCount(1, $result); + $this->assertIsBool($result[0]['result']); + $this->assertTrue($result[0]['result'], 'First polygon’s bounding box should overlap or be to the left of the second’s');Reference for operator behavior. (postgis.net)
52-61: Identical geometries case is appropriate; small clarity tweak and extra assertions.Identical geometries share the same bbox, so the “overlaps” part of
&<makes this TRUE. Add the standard sanity checks and clarify the message to say “same bounding box”.$result = $this->executeDqlQuery($dql); - $this->assertTrue($result[0]['result'], 'Identical geometries should overlap'); + $this->assertCount(1, $result); + $this->assertIsBool($result[0]['result']); + $this->assertTrue($result[0]['result'], 'Identical geometries share the same bounding box and therefore overlap');Operator reference. (postgis.net)
19-61: Optional: add a linestring case to exercise “strictly left without overlap” on non-point primitives.You already cover points (strictly left), polygons (overlap), and identity. To round it off, add a linestring test using row id = 3 to show a non-overlapping, strictly-left scenario on larger primitives.
Example to add:
#[Test] public function overlaps_left_with_disjoint_linestrings(): void { // geometry1: LINESTRING(0 0, 2 2), geometry2: LINESTRING(3 3, 5 5) in fixture row 3 $dql = 'SELECT OVERLAPS_LEFT(g.geometry1, g.geometry2) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 3'; $result = $this->executeDqlQuery($dql); $this->assertCount(1, $result); $this->assertIsBool($result[0]['result']); $this->assertTrue($result[0]['result'], 'First linestring’s bbox is strictly left of the second’s (non-overlapping), so &< is TRUE'); }Reasoning:
&<is TRUE when A’s bbox overlaps OR is to the left of B’s; for row 3 it’s the “left of” case. (postgis.net)
📜 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 (4)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/ORM/Query/AST/Functions/OverlapsAboveTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.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/ORM/Query/AST/Functions/OverlapsAboveTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.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/ORM/Query/AST/Functions/OverlapsAboveTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.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:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php
🧬 Code graph analysis (4)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
OverlapsAbove(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
OverlapsRight(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (4)
Test(19-28)Test(30-39)Test(41-50)Test(52-61)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelow.php (1)
OverlapsBelow(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeft.php (1)
OverlapsLeft(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
⏰ 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). (17)
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- 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.5 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: sloth
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (8)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (4)
12-17: String function registration looks correctOVERLAPS_RIGHT is properly registered and mapped to the OverlapsRight DQL function. Naming and casing align with the usage in the DQL below.
19-28: Good negative case for points (no overlap, left-of)
For POINT(0 0) vs POINT(1 1), OVERLAPS_RIGHT should be false (no overlap and A is left of B). Test reads well.
30-39: Good positive case for points (no overlap, right-of)
Reversing the arguments to assert true matches PostGIS &> semantics (overlaps or is to the right).
52-61: Identical geometries case is correct
Identical bboxes overlap; expecting TRUE is consistent with &> semantics and mirrors the LEFT variant tests.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php (1)
12-17: Function mapping looks correct for &<| (overlaps-or-below).Registration of OVERLAPS_BELOW to OverlapsBelow::class is consistent with the operator prototype '(%s &<| %s)' in the function class. No issues spotted here.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (1)
12-17: Function mapping looks correct and consistent with BaseFunction prototype.Registration via getStringFunctions maps OVERLAPS_ABOVE to OverlapsAbove::class as expected. This will allow DQL to compile to the (%s |&> %s) operator.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php (2)
12-17: Function registration looks correct (DQL keyword ↔ implementation).Mapping 'OVERLAPS_LEFT' to the OverlapsLeft::class, which renders the SQL operator
(%s &< %s), is aligned with PostGIS:&<returns TRUE if A’s bbox overlaps or is to the left of B’s.
Reference for semantics: PostGIS bounding box operators. (postgis.net)
7-8: PHPUnit 10 Attributes Confirmed – No Action RequiredComposer.json specifies PHPUnit "^10.5.53", and all test suites consistently import and use
#[Test]fromPHPUnit\Framework\Attributes\Test, confirming attribute support in PHPUnit 10.
• composer.json —"phpunit/phpunit": "^10.5.53"
• Consistentuse PHPUnit\Framework\Attributes\Test;and#[Test]across unit and integration testsAll tests leverage PHPUnit 10’s attribute-based annotations, so no further changes are needed.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (1)
7-19: Clarify PHPDoc: boolean usage doesn’t have to be compared to TRUE/FALSE.Doctrine can consume the boolean directly (as shown in tests). Suggest wording to avoid “must”.
- * @example Using it in DQL with boolean comparison: "WHERE SPATIAL_SAME(g1.geometry, g2.geometry) = TRUE" - * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. + * Examples: + * - WHERE SPATIAL_SAME(g1.geometry, g2.geometry) = TRUE + * - SELECT SPATIAL_SAME(g1.geometry, g2.geometry) AS result + * Returns boolean; explicit comparison (= TRUE/FALSE) is optional, and may help with older Doctrine versions.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (1)
7-7: Optional: drop unused imports in this suite if the base TestCase already resolves entity class names.Only if other unit tests follow that pattern; otherwise keep as-is for clarity.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (1)
19-50: Optional: add a WHERE-clause usage to mirror the doc example.E.g., assert filtering: “… WHERE SPATIAL_SAME(g.geometry1, g.geometry2) = FALSE”.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
7-20: Docblock: align usage guidance with actual supportDocs say “must be used with = TRUE/FALSE”, but tests use it directly in SELECT and assertions on booleans. Suggest clarifying that explicit comparison is only needed for older Doctrine versions.
- * @example Using it in DQL with boolean comparison: "WHERE SPATIAL_CONTAINED_BY(g1.geometry, g2.geometry) = TRUE" - * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. + * Examples: + * - WHERE SPATIAL_CONTAINED_BY(g1.geometry, g2.geometry) = TRUE (compat with older Doctrine versions) + * - SELECT SPATIAL_CONTAINED_BY(g1.geometry, g2.geometry) (works with current versions; returns boolean) + * Returns boolean.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (1)
19-31: Add a literal-operand case for parity with other operatorsMirror SpatialContains tests by adding a WKT literal case to assert correct parsing of StringPrimary literals.
protected function getExpectedSqlStatements(): array { return [ 'checks if geometry is spatially contained by another' => 'SELECT (c0_.geometry1 @ c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', + 'checks if geometry is spatially contained by literal' => "SELECT (c0_.geometry1 @ 'POINT(1 2)') AS sclr_0 FROM ContainsGeometries c0_", ]; } protected function getDqlStatements(): array { return [ 'checks if geometry is spatially contained by another' => \sprintf('SELECT SPATIAL_CONTAINED_BY(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), + 'checks if geometry is spatially contained by literal' => \sprintf("SELECT SPATIAL_CONTAINED_BY(e.geometry1, 'POINT(1 2)') FROM %s e", ContainsGeometries::class), ]; }tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (1)
12-17: Add a “strictly inside” positive to disambiguate semanticsConsider adding a fixture row where geometry1’s bbox is strictly inside geometry2’s bbox (not identical) and a corresponding positive test. This clarifies inclusive vs. strict behavior beyond identity.
Proposed fixture addition (outside this file) in tests/Integration/.../SpatialOperatorTestCase.php, append to $testData:
[ 'geometry1' => 'POLYGON((1 1, 1 2, 2 2, 2 1, 1 1))', 'geometry2' => 'POLYGON((0 0, 0 3, 3 3, 3 0, 0 0))', 'geography1' => 'SRID=4326;POLYGON((1 1, 1 1.1, 1.1 1.1, 1.1 1, 1 1))', 'geography2' => 'SRID=4326;POLYGON((0 0, 0 2, 2 2, 2 0, 0 0))', ],Then add this test here:
class SpatialContainedByTest extends SpatialOperatorTestCase { @@ #[Test] public function returns_true_when_comparing_identical_geometries(): void { $dql = 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry1) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 1'; $result = $this->executeDqlQuery($dql); $this->assertTrue($result[0]['result']); } + + #[Test] + public function returns_true_with_polygon_strictly_inside(): void + { + $dql = 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry2) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = 4'; + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']); + } }src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (1)
7-20: Doc: relax “must use = TRUE/FALSE” guidance to reflect current DQL usage.The tests demonstrate direct boolean usage in SELECT and WHERE. Suggest softening the wording and adding a second example.
Apply this diff to the docblock:
/** * Implementation of PostGIS spatial bounding box contains operator (using ~). * * Returns TRUE if A's bounding box contains B's. * This is the spatial version of the ~ operator, distinct from the array/JSON version. * * @see https://postgis.net/docs/reference.html#Operators_Geometry * @since 3.5 * * @author Martin Georgiev <martin.georgiev@gmail.com> * - * @example Using it in DQL with boolean comparison: "WHERE SPATIAL_CONTAINS(g1.geometry, g2.geometry) = TRUE" - * Returns boolean, must be used with "= TRUE" or "= FALSE" in DQL. + * @example 1: WHERE SPATIAL_CONTAINS(g1.geometry, g2.geometry) + * @example 2: WHERE SPATIAL_CONTAINS(g1.geometry, g2.geometry) = TRUE + * Returns boolean; it can be used directly in boolean contexts or compared explicitly. */tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (1)
19-33: Add a reversed-operands case to make non-commutativity explicit.Even if it still compiles to valid SQL, capturing the reversed order improves coverage and clarifies semantics.
Apply this diff to add one more assertion pair:
protected function getExpectedSqlStatements(): array { return [ 'checks if geometry spatially contains another' => 'SELECT (c0_.geometry1 ~ c0_.geometry2) AS sclr_0 FROM ContainsGeometries c0_', 'checks if geometry spatially contains literal' => "SELECT (c0_.geometry1 ~ 'POINT(1 2)') AS sclr_0 FROM ContainsGeometries c0_", + 'checks if reversed operands (non-commutative)' => 'SELECT (c0_.geometry2 ~ c0_.geometry1) AS sclr_0 FROM ContainsGeometries c0_', ]; } protected function getDqlStatements(): array { return [ 'checks if geometry spatially contains another' => \sprintf('SELECT SPATIAL_CONTAINS(e.geometry1, e.geometry2) FROM %s e', ContainsGeometries::class), 'checks if geometry spatially contains literal' => \sprintf("SELECT SPATIAL_CONTAINS(e.geometry1, 'POINT(1 2)') FROM %s e", ContainsGeometries::class), + 'checks if reversed operands (non-commutative)' => \sprintf('SELECT SPATIAL_CONTAINS(e.geometry2, e.geometry1) FROM %s e', ContainsGeometries::class), ]; }
📜 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 (9)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/ORM/Query/AST/Functions/SpatialContainsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.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/ORM/Query/AST/Functions/SpatialContainsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.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/ORM/Query/AST/Functions/SpatialContainsTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.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:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php
🧬 Code graph analysis (9)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (1)
SpatialContains(21-29)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (2)
SpatialContainsTest(10-51)getStringFunctions(12-17)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
SpatialContainedBy(21-29)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (2)
SpatialContainedByTest(10-51)getStringFunctions(12-17)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (3)
getStringFunctions(12-17)getExpectedSqlStatements(19-25)getDqlStatements(27-33)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (1)
customizeFunction(23-28)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (1)
customizeFunction(22-27)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (1)
SpatialContains(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (2)
SpatialContainsTest(10-34)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (1)
SpatialSame(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseFunction.php (3)
BaseFunction(20-105)setFunctionPrototype(57-60)addNodeMapping(62-65)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (1)
customizeFunction(23-28)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (1)
customizeFunction(22-27)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
SpatialContainedBy(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (2)
SpatialContainedByTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (1)
SpatialSame(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (2)
SpatialSameTest(10-51)getStringFunctions(12-17)
⏰ 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 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.4
- 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.5 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: sloth
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer latest
- 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.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (14)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSame.php (2)
22-27: Implementation looks correct and consistent with other spatial operators.Prototype and node mappings align with BaseFunction and peers (e.g., SpatialContains/ContainedBy).
13-13: Verify @SInCE value.Ensure 3.5 is the intended release that will ship this operator to keep API docs accurate.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (2)
12-17: Good registration of the DQL function.Mapping SPATIAL_SAME => SpatialSame::class matches library conventions.
19-31: Solid SQL/DQL translation assertions.Covers both DQL and expected SQL with parentheses and operator placement.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialSameTest.php (2)
12-17: Function registration for integration context looks good.Consistent with other integration tests’ setup.
19-50: Nice positive/negative coverage for bbox equality.Covers identical geom and differing/overlapping cases; assertions read clearly.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
23-28: Operator mapping and node parsing look correctPrototype '(%s @ %s)' matches PostGIS bbox “contained by”. Using StringPrimary for both operands is consistent with sibling operators.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (1)
12-17: Registration LGTMDQL function is correctly registered for the test harness.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (3)
19-28: Overlapping polygons case: good negativeValidates non-containment when bboxes overlap but are not nested.
30-39: Separate points case: good negativeCovers disjoint point bboxes.
41-50: Identical geometries case: good positiveConfirms inclusive semantics (A contained by A).
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContains.php (1)
23-28: Boolean bbox operator wired correctly to BaseFunction.Prototype, node mappings, and parentheses match the established pattern (e.g., SpatialSame/ContainedBy). Good to ship.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (1)
10-17: LGTM: function registration matches production class.String function map is correct and consistent with other spatial operator tests.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainsTest.php (1)
10-50: Integration coverage looks solid for bbox semantics.Three representative cases (points, overlapping polygons, identical geometries) pass and align with operator intent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LnTest.php (1)
19-25: Clarify naming: use "Euler's number" instead of ambiguous "Euler constant"; also tighten the literal."Euler constant" is commonly the Euler–Mascheroni constant γ, not e. Rename for clarity and use a more precise literal.
- public function can_calculate_natural_logarithm_of_euler_constant(): void + public function can_calculate_natural_logarithm_of_eulers_number(): void @@ - $dql = 'SELECT LN(2.718281828459) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsNumerics t WHERE t.id = 1'; + $dql = 'SELECT LN(2.718281828459045) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsNumerics t WHERE t.id = 1';tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (4)
20-20: Rename test for precision about directionality.“same vertical level” is ambiguous; the check is that the first shape is not above the second.
- public function returns_false_when_polygons_are_at_same_vertical_level(): void + public function returns_false_when_first_polygon_is_not_above_second(): void
22-24: Stabilize boolean scalar mapping with an explicit CASE.Doctrine/pg bools can surface as 't'/'f' strings depending on platform. Wrapping with CASE ensures a true boolean.
- $dql = 'SELECT OVERLAPS_ABOVE(g.geometry1, g.geometry2) as result + $dql = 'SELECT CASE WHEN OVERLAPS_ABOVE(g.geometry1, g.geometry2) THEN true ELSE false END as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 2'; - $dql = "SELECT OVERLAPS_ABOVE(g.geometry1, 'POINT(0 -1)') as result + $dql = "SELECT CASE WHEN OVERLAPS_ABOVE(g.geometry1, 'POINT(0 -1)') THEN true ELSE false END as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 1"; - $dql = 'SELECT OVERLAPS_ABOVE(g.geometry2, g.geometry1) as result + $dql = 'SELECT CASE WHEN OVERLAPS_ABOVE(g.geometry2, g.geometry1) THEN true ELSE false END as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 2';Note: if you ever hit implicit-cast issues with WKT, wrap the literal via ST_GeomFromText('POINT(0 -1)').
Also applies to: 33-35, 44-46
26-27: Add result-size guards for clearer failures.Assert the single-row shape before indexing.
$result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertFalse($result[0]['result']); $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertTrue($result[0]['result']); $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertTrue($result[0]['result']);Also applies to: 37-38, 48-49
1-51: Optional: Add a “strictly below” negative test for OverlapsAbove
To strengthen spatial-operator coverage, include a complementary scenario where the first geometry is strictly below and disjoint, verifying that OVERLAPS_ABOVE returns false.• File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (after the existing #[Test] methods)
• Suggested snippet:+ #[Test] + public function returns_false_when_first_geometry_is_strictly_below_and_disjoint(): void + { + $dql = 'SELECT OVERLAPS_ABOVE(g.geometryBelow, g.geometryAbove) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE g.id = <fixture-id-for-below-geometry>'; + + $result = $this->executeDqlQuery($dql); + $this->assertFalse($result[0]['result']); + }
📜 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 (6)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LnTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalOverlapsTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NDimensionalCentroidDistanceTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BoundingBoxDistanceTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/GeometryDistanceTest.php
🧰 Additional context used
🧠 Learnings (3)
📚 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/ORM/Query/AST/Functions/LnTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.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/ORM/Query/AST/Functions/LnTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.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/ORM/Query/AST/Functions/LnTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php
🧬 Code graph analysis (2)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LnTest.php (1)
tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAbove.php (1)
OverlapsAbove(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
⏰ 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 + 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.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- 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 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: sloth
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (4)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LnTest.php (1)
27-33: Rename reads well and aligns with the suite’s descriptive style.No functional changes; method name is clear and consistent.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsAboveTest.php (3)
12-17: DQL function registration looks correct.Key matches the DQL token and maps to the expected class. No issues.
30-39: Test logic reads well and matches the intended operator semantics.Good use of a literal point to validate “above”.
41-50: Positive case for polygon vs polygon is clear and consistent.Covers the directionality of the operator.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.php (1)
19-25: Prefer “Euler’s number” over “Euler constant” in test name“Euler’s constant” typically means the Euler–Mascheroni constant (γ), not e. Rename to avoid ambiguity.
- public function can_calculate_exponential_of_one_to_euler_constant(): void + public function can_calculate_exp_of_one_equals_eulers_number(): voidtests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LogTest.php (1)
7-11: Optional: add explicit coverage attribute and a single-argument LOG case
- Add #[CoversClass(Log::class)] for precise mapping in coverage reports.
- Consider a single-argument LOG test to guard both forms supported by the DQL function.
Example additions:
use PHPUnit\Framework\Attributes\CoversClass; #[CoversClass(Log::class)] class LogTest extends NumericTestCase { // ... #[Test] public function can_calculate_base_ten_logarithm_with_single_argument(): void { $dql = 'SELECT LOG(100) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsNumerics t WHERE t.id = 1'; $result = $this->executeDqlQuery($dql); $this->assertEqualsWithDelta(2.0, $result[0]['result'], 0.000001); } }
📜 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)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LogTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsBelowTest.php
🧰 Additional context used
🧠 Learnings (4)
📚 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/ORM/Query/AST/Functions/LogTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.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/ORM/Query/AST/Functions/LogTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.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/ORM/Query/AST/Functions/LogTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.php
📚 Learning: 2025-05-23T11:24:46.898Z
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).
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LogTest.php
🧬 Code graph analysis (2)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LogTest.php (1)
tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.php (1)
tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
⏰ 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.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + 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 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- 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.1 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: sloth
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (3)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ExpTest.php (1)
27-33: LGTM: clearer, behavior-oriented test nameThe rename improves readability and intent.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/LogTest.php (2)
19-25: Descriptive test name: good renameClearer intent without changing behavior. Assertions and DQL remain correct for LOG(10, 100) → 2.0.
27-33: Descriptive test name: good renameMatches existing expectation for LOG(10, n.decimal1). Stable delta choice.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (1)
41-50: Fix expectation: OVERLAPS_RIGHT (&>) is TRUE when bboxes overlapPer PostGIS, A &> B returns TRUE if A’s bbox overlaps or is to the right of B’s bbox (i.e., not strictly left). For id=2, the polygons’ bboxes overlap horizontally, so the result should be TRUE, not FALSE. (postgis.net)
Apply this diff to correct the test and name:
- public function returns_false_when_polygons_overlap_without_right_positioning(): void + public function returns_true_when_polygons_overlap(): void @@ - $result = $this->executeDqlQuery($dql); - $this->assertFalse($result[0]['result']); + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']);
🧹 Nitpick comments (3)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (3)
22-24: Force boolean typing in results with “= TRUE” to avoid driver/db inconsistencies.Adding “= TRUE” yields a plain boolean scalar, preventing cases where DBAL returns 't'/'f' strings.
- $dql = 'SELECT STRICTLY_RIGHT(g.geometry1, g.geometry2) as result + $dql = 'SELECT STRICTLY_RIGHT(g.geometry1, g.geometry2) = TRUE as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 1'; @@ - $dql = 'SELECT STRICTLY_RIGHT(g.geometry2, g.geometry1) as result + $dql = 'SELECT STRICTLY_RIGHT(g.geometry2, g.geometry1) = TRUE as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 3';Also applies to: 44-46
7-9: Prefer ::class over hard-coded FQCN strings in DQL to avoid drift.Import the entity and build the DQL with ContainsGeometries::class. Cleaner and safer.
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\StrictlyRight; use PHPUnit\Framework\Attributes\Test; +use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries; @@ - $dql = 'SELECT STRICTLY_RIGHT(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT STRICTLY_RIGHT(g.geometry1, g.geometry2) as result FROM %s g WHERE g.id = 1', + ContainsGeometries::class, + ); @@ - $dql = 'SELECT STRICTLY_RIGHT(g.geometry2, g.geometry1) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 3'; + $dql = \sprintf( + 'SELECT STRICTLY_RIGHT(g.geometry2, g.geometry1) as result FROM %s g WHERE g.id = 3', + ContainsGeometries::class, + );Also applies to: 22-24, 44-46
42-42: Fix typo in test name.“onetrajetoryds” → clearer wording reads “lower_linestring”.
- public function returns_true_when_higher_linestring_is_strictly_to_the_right_of_lower_onetrajetoryds(): void + public function returns_true_when_higher_linestring_is_strictly_to_the_right_of_lower_linestring(): void
📜 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 (6)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#0
File: :0-0
Timestamp: 2025-08-25T21:35:48.228Z
Learning: Extensions in integration tests should be created in the createTestSchema() method of the base TestCase, following the established PostGIS pattern with proper error handling, not in individual test createXExtension() methods.
📚 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/ORM/Query/AST/Functions/StrictlyRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.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/ORM/Query/AST/Functions/StrictlyRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.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/ORM/Query/AST/Functions/StrictlyRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.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:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php
📚 Learning: 2025-08-26T22:06:36.620Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#430
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php:41-50
Timestamp: 2025-08-26T22:06:36.620Z
Learning: The PostGIS OVERLAPS_RIGHT (&>) operator returns TRUE only if the left geometry's bounding box overlaps or is entirely to the right of the right geometry's bounding box. It returns FALSE if the left geometry extends to the left of the right geometry's leftmost boundary, even if the geometries overlap.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php
🧬 Code graph analysis (2)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
StrictlyRight(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (2)
StrictlyRightTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRight.php (1)
OverlapsRight(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
⏰ 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: sloth
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + 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 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (6)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php (4)
12-17: LGTM: function mapping registered correctlyMapping OVERLAPS_RIGHT to the &> prototype is consistent with the implementation in OverlapsRight.php.
19-28: LGTM: left-of case correctly asserts FALSEPoint at x=0 vs x=1 should yield FALSE for &>. Solid sanity check.
30-39: LGTM: reversed points assert TRUEReversing arguments validates the right-of path. Good coverage symmetry.
52-61: LGTM: identical geometries assert TRUESelf-comparison should be TRUE; this anchors bbox-overlap semantics.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (2)
10-17: Solid test skeleton and DQL function wiring.Extends the right base, registers STRICTLY_RIGHT correctly. Good foundation.
10-17: No changes needed for PostGIS setup
ThecreateTestSchema()method in the baseTestCase(invoked bySpatialOperatorTestCase) already handles creating the PostGIS extension and setting the proper search path, so your newStrictlyRightTestwill have GEOMETRY/GEOGRAPHY support out of the box.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (1)
22-24: Good: operands are typed columns, avoiding the prior “unknown << unknown” trap.Using g.geometry1/2 ensures proper operator overload resolution.
🧹 Nitpick comments (11)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (2)
22-24: Prefer ::class over hard-coded FQCN strings in DQLUsing the FQCN constant prevents typos and eases refactors. Replace inline strings with
ContainsGeometries::class.Apply this diff to the three DQL statements:
- $dql = 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 2'; + $dql = \sprintf( + 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry2) as result FROM %s g WHERE g.id = 2', + ContainsGeometries::class, + ); - $dql = 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry2) as result FROM %s g WHERE g.id = 1', + ContainsGeometries::class, + ); - $dql = 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry1) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT SPATIAL_CONTAINED_BY(g.geometry1, g.geometry1) as result FROM %s g WHERE g.id = 1', + ContainsGeometries::class, + );Also add this import (outside the ranges above):
use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries;Also applies to: 33-35, 44-46
26-28: Add a quick guard on result cardinalityAssert the query returns exactly one row before checking the boolean; this makes failures easier to diagnose.
- $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertFalse($result[0]['result']);- $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertFalse($result[0]['result']);- $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertTrue($result[0]['result']);Also applies to: 37-39, 48-50
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (1)
10-11: Optional: add one WHERE-usage test to codify boolean usage pattern.Prior learnings recommend using operator-like functions as booleans with “= TRUE”. Add a small query exercising WHERE to guard regressions.
@@ class StrictlyLeftTest extends SpatialOperatorTestCase { @@ public function returns_false_with_identical_geometries(): void { $dql = 'SELECT STRICTLY_LEFT(g.geometry1, g.geometry1) as result FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g WHERE g.id = 1'; $result = $this->executeDqlQuery($dql); $this->assertFalse($result[0]['result']); } + + #[Test] + public function works_in_where_clause_with_boolean_comparison(): void + { + $dql = 'SELECT g.id as id + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g + WHERE STRICTLY_LEFT(g.geometry1, g.geometry2) = TRUE AND g.id = 1'; + $result = $this->executeDqlQuery($dql); + $this->assertSame(1, (int) $result[0]['id']); + } }tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (3)
7-9: Prefer ::class over hard-coded FQCNs in DQL.Reduces brittleness on refactors and aligns with the Unit test style using ContainsGeometries::class.
Apply:
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\StrictlyRight; +use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries; use PHPUnit\Framework\Attributes\Test; @@ - $dql = 'SELECT STRICTLY_RIGHT(g.geometry1, g.geometry2) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1'; + $dql = \sprintf( + 'SELECT STRICTLY_RIGHT(g.geometry1, g.geometry2) as result FROM %s g WHERE g.id = 1', + ContainsGeometries::class + ); @@ - $dql = "SELECT STRICTLY_RIGHT(g.geometry1, 'POINT(-5 -5)') as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1"; + $dql = \sprintf( + "SELECT STRICTLY_RIGHT(g.geometry1, 'POINT(-5 -5)') as result FROM %s g WHERE g.id = 1", + ContainsGeometries::class + ); @@ - $dql = 'SELECT STRICTLY_RIGHT(g.geometry2, g.geometry1) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 3'; + $dql = \sprintf( + 'SELECT STRICTLY_RIGHT(g.geometry2, g.geometry1) as result FROM %s g WHERE g.id = 3', + ContainsGeometries::class + );Also applies to: 22-24, 33-35, 44-46
27-27: Cast DB result to bool to avoid driver-dependent truthiness.Some drivers may return 't'/'f' strings; explicit cast keeps assertions stable.
- $this->assertFalse($result[0]['result']); + $this->assertFalse((bool) $result[0]['result']); @@ - $this->assertTrue($result[0]['result']); + $this->assertTrue((bool) $result[0]['result']); @@ - $this->assertTrue($result[0]['result']); + $this->assertTrue((bool) $result[0]['result']);Also applies to: 38-38, 49-49
42-42: Fix typos in test name.Clarify and correct spelling for discoverability and grep-ability.
- public function returns_true_when_higher_linestring_is_strictly_to_the_right_of_lower_onetrajetoryds(): void + public function returns_true_when_linestring_is_strictly_to_the_right_of_another_linestring(): voidtests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (5)
7-9: Import the fixture entity and prefer ::class to avoid brittle FQCN stringsUsing the class constant reduces typos and escaping pitfalls in the DQL.
use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TrajectoryDistance; use PHPUnit\Framework\Attributes\Test; +use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries;
22-24: Use ::class and prefer LINESTRINGM WKT for broader PostGIS compatibility
- Swaps string FQCNs for ContainsGeometries::class.
- Uses LINESTRINGM(...) (WKT/EWKT style many installations accept consistently).
- $dql = "SELECT TRAJECTORY_DISTANCE('LINESTRING M(0 0 1, 1 1 2)', 'LINESTRING M(2 2 1, 3 3 2)') as distance - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1"; + $dql = \sprintf( + "SELECT TRAJECTORY_DISTANCE('LINESTRINGM(0 0 1, 1 1 2)', 'LINESTRINGM(2 2 1, 3 3 2)') AS distance FROM %s g WHERE g.id = 1", + ContainsGeometries::class + );- $dql = "SELECT TRAJECTORY_DISTANCE('LINESTRING M(0 0 1, 1 1 2)', 'LINESTRING M(0 0 1, 1 1 2)') as distance - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1"; + $dql = \sprintf( + "SELECT TRAJECTORY_DISTANCE('LINESTRINGM(0 0 1, 1 1 2)', 'LINESTRINGM(0 0 1, 1 1 2)') AS distance FROM %s g WHERE g.id = 1", + ContainsGeometries::class + );- $dql = "SELECT TRAJECTORY_DISTANCE('LINESTRING M(0 0 0, 5 5 10)', 'LINESTRING M(1 1 0, 6 6 10)') as distance - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsGeometries g - WHERE g.id = 1"; + $dql = \sprintf( + "SELECT TRAJECTORY_DISTANCE('LINESTRINGM(0 0 0, 5 5 10)', 'LINESTRINGM(1 1 0, 6 6 10)') AS distance FROM %s g WHERE g.id = 1", + ContainsGeometries::class + );Also applies to: 34-36, 45-47
26-29: Guard result cardinality before indexingFail fast if the query unexpectedly returns 0 or >1 rows.
- $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertIsNumeric($result[0]['distance']); $this->assertGreaterThanOrEqual(0, $result[0]['distance']);- $result = $this->executeDqlQuery($dql); + $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); $this->assertIsNumeric($result[0]['distance']); $this->assertGreaterThanOrEqual(0, $result[0]['distance']);Also applies to: 49-52
31-40: Use delta-based assertion for floating-point robustnessAvoid fragile exact-equality on DB floating results.
- $result = $this->executeDqlQuery($dql); - $this->assertEquals(0, $result[0]['distance']); + $result = $this->executeDqlQuery($dql); + $this->assertCount(1, $result); + $this->assertEqualsWithDelta(0.0, (float) $result[0]['distance'], 1.0e-9);
53-53: Optional: add a symmetry property testDistance should be symmetric; this increases confidence in the operator mapping.
#[Test] public function trajectory_distance_is_symmetric(): void { $dql = \sprintf( "SELECT TRAJECTORY_DISTANCE('LINESTRINGM(0 0 0, 1 1 1)', 'LINESTRINGM(2 2 0, 3 3 1)') AS d1, TRAJECTORY_DISTANCE('LINESTRINGM(2 2 0, 3 3 1)', 'LINESTRINGM(0 0 0, 1 1 1)') AS d2 FROM %s g WHERE g.id = 1", ContainsGeometries::class ); $result = $this->executeDqlQuery($dql); $this->assertCount(1, $result); $this->assertEqualsWithDelta((float)$result[0]['d1'], (float)$result[0]['d2'], 1.0e-9); }
📜 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 (6)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsLeftTest.php
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php
🧰 Additional context used
🧠 Learnings (6)
📚 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/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.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/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.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/ORM/Query/AST/Functions/SpatialContainedByTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.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:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.phptests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.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/ORM/Query/AST/Functions/StrictlyLeftTest.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/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php
🧬 Code graph analysis (4)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedBy.php (1)
SpatialContainedBy(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (2)
SpatialContainedByTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (4)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeft.php (1)
StrictlyLeft(20-28)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistance.php (1)
TrajectoryDistance(21-29)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
TrajectoryDistanceTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRight.php (1)
StrictlyRight(20-28)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (2)
StrictlyRightTest(10-32)getStringFunctions(12-17)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialOperatorTestCase.php (1)
SpatialOperatorTestCase(9-80)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(271-285)
🔇 Additional comments (13)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/SpatialContainedByTest.php (4)
12-17: Function registration looks correct
SPATIAL_CONTAINED_BYis properly mapped toSpatialContainedBy::class. No issues.
19-28: Test scenario reads correctly for bbox “contained by” semanticsOverlapping polygons (id=2) should not satisfy
@; expectation is correct.
30-39: Point vs point comparison is appropriateSeparate points (id=1) correctly yield false for
@. Looks good.
41-50: Identity case is a solid positive controlSelf-comparison returns true as expected. Nice coverage.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyLeftTest.php (5)
12-17: DQL mapping is correct and consistent.Registers STRICTLY_LEFT → StrictlyLeft::class as expected for tests.
22-28: Happy path is valid (POINT(0,0) << POINT(1,1) → true).Asserts align with bbox semantics of <<.
33-39: Overlap case correctly returns false.Row 2 polygons’ bboxes intersect; expectation is correct.
44-50: Reversed operands case is correct.POINT(1,1) is not strictly left of POINT(0,0).
55-61: Identical geometries case is correct.Same bbox ⇒ not strictly left.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php (2)
12-17: Function mapping looks correct.Registering STRICTLY_RIGHT to the AST class mirrors the other spatial operator tests.
8-8: No action needed: PHPUnit ≥10 is already enforcedThe project’s root composer.json defines PHPUnit at
^10.5.53, which satisfies the requirement for attribute-based tests and ensures the[Test]attribute is supported in CI/runtime. Tests annotated with#[Test]will not be skipped.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (2)
12-17: Function registration LGTMCorrectly registers the DQL function mapping for the operator.
22-24: Please verify WKT “M”‐dimensional variant support across your PostGIS matrixBefore merging, ensure that all PostGIS versions you officially support accept the spaced syntax (
LINESTRING M(...)). If any older versions require the no‐space form (LINESTRINGM(...)), you’ll need to either normalize inputs in your code or adjust your test fixtures accordingly.Key locations using the spaced form:
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TrajectoryDistanceTest.php (lines 22, 34, 45)
- Numerous Unit tests under tests/Unit/MartinGeorgiev/** using
WktSpatialData::fromWkt('LINESTRING M(...)')- Documentation:
- docs/SPATIAL-TYPES.md
- docs/USE-CASES-AND-EXAMPLES.md
- docs/GEOMETRY-ARRAYS.md
You can search for both variants in your repo with:
rg -nP "LINESTRING\s+M\(" -g '!vendor/**' rg -nP "LINESTRINGM\s*\(" -g '!vendor/**'If you discover any unsupported cases, update either your code (to normalize or reject) or your tests to use the universally supported form.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/USE-CASES-AND-EXAMPLES.md (1)
139-141: Align range example with the boolean-function pattern used throughout.DQL does not accept custom operators like @>. Use the registered
CONTAINS(...) = TRUEform.-SELECT p FROM Product p WHERE p.priceRange @> 25.0 +SELECT p FROM Product p WHERE CONTAINS(p.priceRange, 25.0) = TRUE
🧹 Nitpick comments (5)
README.md (2)
77-82: Clarify that these are bounding-box operators and link to the new doc section.Readers may assume “spatial containment” means exact topological relationships. These bullets list PostGIS bbox operators. Suggest making that explicit and linking to the dedicated section for usage and boolean semantics.
- - Spatial containment (`@`, `~`) + - Bounding-box containment (`@`, `~`)And after the list, add:
+See “PostGIS Spatial Operators” for DQL usage and = TRUE semantics: docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md#postgis-spatial-operators.
86-86: Make function-name capitalization consistent across docs.Elsewhere you register DQL names as ILIKE/SIMILAR_TO. Either standardize here to uppercase or confirm Doctrine registration is case-insensitive and note it.
- - Pattern matching (`ilike`, `similar to`) + - Pattern matching (`ILIKE`, `SIMILAR TO`)docs/USE-CASES-AND-EXAMPLES.md (1)
248-252: Avoid implying ST_Intersects works in DQL without registration.Unless ST_Intersects is registered in this library, this line won’t parse in DQL. Either (a) show a second DQL function you provide for exact checks, or (b) explicitly mark this as native SQL or “if you register ST_Intersects”.
- AND ST_Intersects(e.geometry, 'POLYGON((0 0, 10 10, 20 20, 0 0))') -- Exact check + -- Exact check (native SQL or a custom-registered DQL function, e.g. ST_INTERSECTS): + AND ST_Intersects(e.geometry, 'POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))')docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md (2)
96-104: Verify operator semantics for<@>and|=|rows.
<@>is often tied to earthdistance/cube, not core PostGIS;|=|(trajectory distance) is advanced/extension-specific. Confirm these are actually implemented and covered by tests in this repo; otherwise mark as “not available” or move to an “extensions” note.
15-20: Strengthen the boolean-operator guideline.Use “must” for normative tone used elsewhere.
-- **Boolean operators**: All spatial operators return boolean values and **shall be used with `= TRUE` or `= FALSE` in DQL** +- **Boolean operators**: All spatial operators return boolean values and **must be used with `= TRUE` or `= FALSE` in DQL**
📜 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)
README.md(1 hunks)docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md(3 hunks)docs/USE-CASES-AND-EXAMPLES.md(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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:
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.mdREADME.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:
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.mddocs/USE-CASES-AND-EXAMPLES.mdREADME.md
📚 Learning: 2025-08-27T18:19:35.778Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#430
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/StrictlyRightTest.php:31-36
Timestamp: 2025-08-27T18:19:35.778Z
Learning: WKT string literals can be used directly in Doctrine spatial function tests (e.g., 'POINT(-5 -5)') without explicit ST_GeomFromText wrapping, as PostgreSQL/PostGIS automatically handles the implicit conversion to geometry types when used with spatial operators.
Applied to files:
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.mddocs/USE-CASES-AND-EXAMPLES.md
📚 Learning: 2025-08-26T22:06:36.620Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#430
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsRightTest.php:41-50
Timestamp: 2025-08-26T22:06:36.620Z
Learning: The PostGIS OVERLAPS_RIGHT (&>) operator returns TRUE only if the left geometry's bounding box overlaps or is entirely to the right of the right geometry's bounding box. It returns FALSE if the left geometry extends to the left of the right geometry's leftmost boundary, even if the geometries overlap.
Applied to files:
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.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/AVAILABLE-FUNCTIONS-AND-OPERATORS.mdREADME.md
🪛 LanguageTool
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...e | Spatial Usage | Text/Pattern Usage | |---|---|---|---| | @> | CONTAINS (a...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...| Text/Pattern Usage | |---|---|---|---| | @> | CONTAINS (arrays contain elem...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...atically with geometry/geography | N/A | | <@ | IS_CONTAINED_BY (element in a...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...atically with geometry/geography | N/A | | @ | N/A | SPATIAL_CONTAINED_BY (bo...
(QB_NEW_EN)
[grammar] ~11-11: There might be a mistake here.
Context: ...NED_BY(bounding box contained) | N/A | || N/A |SPATIAL_CONTAINS` (boundi...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...ns) | REGEXP (text pattern matching) | | && | OVERLAPS (arrays/ranges overl...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...y/geography | N/A | Usage Guidelines: - Arrays/JSON: Use CONTAINS, `IS_CONTA...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...y/geography 📝 Compatibility Notes: - Most bounding box operators work primari...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...s work primarily with geometry types - Geography types have limited operator ...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...ator support (mainly &&, <->, <@>) - 3D/n-dimensional operators may require...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...not be available in all PostGIS versions ### Bounding Box Operators These operators ...
(QB_NEW_EN)
[grammar] ~230-~230: There might be a mistake here.
Context: ...| Register for DQL as | Implemented by | |---|---|---| | array | ARRAY | `MartinG...
(QB_NEW_EN)
README.md
[grammar] ~77-~77: There might be a mistake here.
Context: ...ps (&&) - PostGIS Spatial Operations - Bounding box relationships (<<, >>, ...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ... &<, &>, |&>, &<|, <<|, |>>) - Spatial containment (@, ~) - Dista...
(QB_NEW_EN)
[grammar] ~79-79: There might be a mistake here.
Context: ...|>>) - Spatial containment (@, ) - Distance calculations (<->, <#>, <<...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ... (<->, <#>, <<->>, <<#>>, |=|) - N-dimensional operations (&&&) ### Fu...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...sional operations (&&&) ### Functions - Text Search - Full text search (`to_...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...s (&&&) ### Functions - Text Search - Full text search (to_tsvector, `to_tsq...
(QB_NEW_EN)
[uncategorized] ~85-~85: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...) ### Functions - **Text Search** - Full text search (to_tsvector, to_tsquery`) ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~85-~85: There might be a mistake here.
Context: ...ext search (to_tsvector, to_tsquery) - Pattern matching (ilike, similar to)...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...Pattern matching (ilike, similar to) - Regular expressions - *Array Functions...
(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: sloth
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 16 + 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.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 1.2
- 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 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (2)
docs/USE-CASES-AND-EXAMPLES.md (1)
183-184: Nice: upfront reminder that spatial operators must be compared to TRUE/FALSE in DQL.docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md (1)
63-79: Good breadth and correct escaping of vertical/horizontal operators.
| | `@>` | `CONTAINS` (arrays contain elements) | Works automatically with geometry/geography | N/A | | ||
| | `<@` | `IS_CONTAINED_BY` (element in array) | Works automatically with geometry/geography | N/A | | ||
| | `@` | N/A | `SPATIAL_CONTAINED_BY` (bounding box contained) | N/A | | ||
| | `~` | N/A | `SPATIAL_CONTAINS` (bounding box contains) | `REGEXP` (text pattern matching) | | ||
| | `&&` | `OVERLAPS` (arrays/ranges overlap) | Works automatically with geometry/geography | N/A | |
There was a problem hiding this comment.
Correct spatial-usage cells for @> and <@ (they don’t apply to PostGIS bbox semantics).
Listing them as “Works automatically with geometry/geography” is misleading; PostGIS uses @ and ~ for bbox contain/contained-by. Keep these N/A under Spatial.
-| `@>` | `CONTAINS` (arrays contain elements) | Works automatically with geometry/geography | N/A |
-| `<@` | `IS_CONTAINED_BY` (element in array) | Works automatically with geometry/geography | N/A |
+| `@>` | `CONTAINS` (arrays contain elements) | N/A | N/A |
+| `<@` | `IS_CONTAINED_BY` (element in array) | N/A | N/A |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `@>` | `CONTAINS` (arrays contain elements) | Works automatically with geometry/geography | N/A | | |
| | `<@` | `IS_CONTAINED_BY` (element in array) | Works automatically with geometry/geography | N/A | | |
| | `@` | N/A | `SPATIAL_CONTAINED_BY` (bounding box contained) | N/A | | |
| | `~` | N/A | `SPATIAL_CONTAINS` (bounding box contains) | `REGEXP` (text pattern matching) | | |
| | `&&` | `OVERLAPS` (arrays/ranges overlap) | Works automatically with geometry/geography | N/A | | |
| | `@>` | `CONTAINS` (arrays contain elements) | N/A | N/A | | |
| | `<@` | `IS_CONTAINED_BY` (element in array) | N/A | N/A | | |
| | `@` | N/A | `SPATIAL_CONTAINED_BY` (bounding box contained) | N/A | | |
| | `~` | N/A | `SPATIAL_CONTAINS` (bounding box contains) | `REGEXP` (text pattern matching) | | |
| | `&&` | `OVERLAPS` (arrays/ranges overlap) | Works automatically with geometry/geography | N/A | |
🧰 Tools
🪛 LanguageTool
[grammar] ~9-~9: There might be a mistake here.
Context: ...atically with geometry/geography | N/A | | <@ | IS_CONTAINED_BY (element in a...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...atically with geometry/geography | N/A | | @ | N/A | SPATIAL_CONTAINED_BY (bo...
(QB_NEW_EN)
[grammar] ~11-11: There might be a mistake here.
Context: ...NED_BY(bounding box contained) | N/A | || N/A |SPATIAL_CONTAINS` (boundi...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...ns) | REGEXP (text pattern matching) | | && | OVERLAPS (arrays/ranges overl...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md around lines 9-13, the
spatial-usage cells for the `@>` and `<@` rows are incorrect; change their
Spatial column entries from "Works automatically with geometry/geography" to
"N/A" (since PostGIS bbox semantics use `@` and `~`), verify that the `@` row is
labeled `SPATIAL_CONTAINED_BY` and the `~` row is labeled `SPATIAL_CONTAINS`,
and ensure the table consistently marks only `@` and `~` as the spatial bbox
operators while keeping `@>` and `<@` as array operators only.
| - Most bounding box operators work primarily with **geometry** types | ||
| - **Geography** types have limited operator support (mainly `&&`, `<->`, `<@>`) | ||
| - **3D/n-dimensional operators** may require explicit type casting: `ST_GeomFromText('POINT Z(0 0 0)')` | ||
| - Some advanced operators (`&&&`, `<<#>>`) may not be available in all PostGIS versions |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Tighten compatibility notes for geography and remove likely-incorrect <@> reference.
Geography reliably supports && and KNN <-> in recent PostGIS; <@> is not a PostGIS geography operator.
-- **Geography** types have limited operator support (mainly `&&`, `<->`, `<@>`)
+- **Geography** types have limited operator support (commonly `&&`, `<->` with KNN GiST/SP-GiST)Update geography compatibility notes
Based on the PostGIS documentation, geography columns support only the standard bounding-box intersection operator and KNN distance operator, not the <@> containment operator:
- The
&&operator is defined for both geometry and geography types (stackoverflow.com) - The
<->operator provides KNN (“K nearest neighbour”) distance ordering for geography as well as geometry (postgis.net) - The
@(and any variant like<@>) operator is only defined on geometry types; geography does not support it (access.crunchydata.com, stackoverflow.com)
Please apply this change to docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md (around lines 55–58):
- **Geography** types have limited operator support (mainly `&&`, `<->`, `<@>`)
+ **Geography** types have limited operator support (commonly `&&`, `<->` with KNN GiST/SP-GiST)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **📝 Compatibility Notes**: | |
| - Most bounding box operators work primarily with **geometry** types | |
| - **Geography** types have limited operator support (mainly `&&`, `<->`, `<@>`) | |
| - **3D/n-dimensional operators** may require explicit type casting: `ST_GeomFromText('POINT Z(0 0 0)')` | |
| **📝 Compatibility Notes**: | |
| - Most bounding box operators work primarily with **geometry** types | |
| - **Geography** types have limited operator support (commonly `&&`, `<->` with KNN GiST/SP-GiST) | |
| - **3D/n-dimensional operators** may require explicit type casting: `ST_GeomFromText('POINT Z(0 0 0)')` |
🧰 Tools
🪛 LanguageTool
[grammar] ~55-~55: There might be a mistake here.
Context: ...y/geography 📝 Compatibility Notes: - Most bounding box operators work primari...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...s work primarily with geometry types - Geography types have limited operator ...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...ator support (mainly &&, <->, <@>) - 3D/n-dimensional operators may require...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md around lines 55 to 58, update the
geography compatibility notes to reflect PostGIS docs: state that geography
supports the bounding-box intersection operator (`&&`) and the KNN distance
operator (`<->` but not the containment operator variant like `<@>` or `@`), and
remove or replace the incorrect claim that `<@>` is supported for geography;
edit the three bullet points so they explicitly list `&&` and `<->` for
geography and note that containment operators (e.g., `<@>`, `@`) are
geometry-only.
| PostGIS spatial operators allow you to perform spatial queries using bounding box relationships and distance calculations. **Important**: All spatial operators return boolean values and shall be used with `= TRUE` or `= FALSE` in DQL. | ||
|
|
||
| #### Bounding Box Spatial Relationships | ||
|
|
||
| ```sql | ||
| -- Find geometries to the left of a reference point | ||
| SELECT e FROM Entity e WHERE STRICTLY_LEFT(e.geometry, 'POINT(0 0)') = TRUE | ||
|
|
||
| -- Find geometries that spatially contain a point (bounding box level) | ||
| SELECT e FROM Entity e WHERE SPATIAL_CONTAINS(e.polygon, 'POINT(1 1)') = TRUE | ||
|
|
||
| -- Find geometries contained within a bounding box | ||
| SELECT e FROM Entity e WHERE SPATIAL_CONTAINED_BY(e.geometry, 'POLYGON((0 0, 10 10, 20 20, 0 0))') = TRUE | ||
|
|
||
| -- Check if two geometries have the same bounding box | ||
| SELECT e FROM Entity e WHERE SPATIAL_SAME(e.geometry1, e.geometry2) = TRUE | ||
|
|
||
| -- Vertical relationships | ||
| SELECT e FROM Entity e WHERE STRICTLY_ABOVE(e.geometry, 'LINESTRING(0 0, 5 0)') = TRUE | ||
| SELECT e FROM Entity e WHERE OVERLAPS_BELOW(e.geometry, 'POLYGON((0 5, 5 5, 5 10, 0 10, 0 5))') = TRUE | ||
|
|
||
| -- 3D spatial relationships | ||
| SELECT e FROM Entity e WHERE ND_OVERLAPS(e.geometry3d, 'POLYGON Z((0 0 0, 1 1 1, 2 2 2, 0 0 0))') = TRUE | ||
| ``` |
There was a problem hiding this comment.
Fix invalid/degenerate WKT in examples.
POLYGON((0 0, 10 10, 20 20, 0 0)) is colinear and invalid. Provide valid rings; also prefer simple rectangles for clarity. The 3D polygon example is likewise degenerate.
-SELECT e FROM Entity e WHERE SPATIAL_CONTAINED_BY(e.geometry, 'POLYGON((0 0, 10 10, 20 20, 0 0))') = TRUE
+SELECT e FROM Entity e WHERE SPATIAL_CONTAINED_BY(e.geometry, 'POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))') = TRUE
@@
-SELECT e FROM Entity e WHERE ND_OVERLAPS(e.geometry3d, 'POLYGON Z((0 0 0, 1 1 1, 2 2 2, 0 0 0))') = TRUE
+SELECT e FROM Entity e WHERE ND_OVERLAPS(e.geometry3d, 'POLYGON Z((0 0 0, 0 1 0, 1 1 0, 1 0 0, 0 0 0))') = TRUE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PostGIS spatial operators allow you to perform spatial queries using bounding box relationships and distance calculations. **Important**: All spatial operators return boolean values and shall be used with `= TRUE` or `= FALSE` in DQL. | |
| #### Bounding Box Spatial Relationships | |
| ```sql | |
| -- Find geometries to the left of a reference point | |
| SELECT e FROM Entity e WHERE STRICTLY_LEFT(e.geometry, 'POINT(0 0)') = TRUE | |
| -- Find geometries that spatially contain a point (bounding box level) | |
| SELECT e FROM Entity e WHERE SPATIAL_CONTAINS(e.polygon, 'POINT(1 1)') = TRUE | |
| -- Find geometries contained within a bounding box | |
| SELECT e FROM Entity e WHERE SPATIAL_CONTAINED_BY(e.geometry, 'POLYGON((0 0, 10 10, 20 20, 0 0))') = TRUE | |
| -- Check if two geometries have the same bounding box | |
| SELECT e FROM Entity e WHERE SPATIAL_SAME(e.geometry1, e.geometry2) = TRUE | |
| -- Vertical relationships | |
| SELECT e FROM Entity e WHERE STRICTLY_ABOVE(e.geometry, 'LINESTRING(0 0, 5 0)') = TRUE | |
| SELECT e FROM Entity e WHERE OVERLAPS_BELOW(e.geometry, 'POLYGON((0 5, 5 5, 5 10, 0 10, 0 5))') = TRUE | |
| -- 3D spatial relationships | |
| SELECT e FROM Entity e WHERE ND_OVERLAPS(e.geometry3d, 'POLYGON Z((0 0 0, 1 1 1, 2 2 2, 0 0 0))') = TRUE | |
| ``` | |
| #### Bounding Box Spatial Relationships | |
Summary by CodeRabbit
New Features
Tests
Documentation