fix(#446): restore OVERLAPS to its intended namespace and strengthen its test coverage
#447
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughNamespace for the Overlaps AST function moved from the PostGIS sub-namespace into the parent Functions namespace; imports and test namespaces were updated accordingly. A new integration test suite for the OVERLAPS DQL function was added and existing tests updated to reference the new FQCN. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (18)
✨ 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. Comment |
![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 (5)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php (1)
5-5: Optional: add PostGIS BC shim for OverlapsNo occurrences of MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS\Overlaps were found in the repo (only OverlapsLeft/Right/Above/Below are referenced). Add a lightweight deprecated alias to preserve backward compatibility for external users if desired — e.g. create src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/Overlaps.php:
<?php declare(strict_types=1); namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS; /** * @deprecated Use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Overlaps instead. */ class Overlaps extends \MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Overlaps {}tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php (1)
24-29: Optional: add scenarios for empty and unbounded ranges in subclasses’ providers.OVERLAPS on ranges has nuances with empty/unbounded ends; adding such cases in concrete RangeType tests would strengthen coverage.
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (3)
19-39: Parameterized array comparisons: verify transformer casting across element types.These tests rely on PHPArrayToPostgresValueTransformer; ensure it emits appropriately typed array literals for ints/bools so PG doesn’t need risky implicit casts during &&.
You can sanity-check with a query log or add one more test that uses an explicit typed literal to prove parity:
+ #[Test] + public function returns_true_with_explicitly_typed_integer_array_literal(): void + { + $dql = "SELECT OVERLAPS(t.integerArray, 'ARRAY[2,5,6]'::integer[]) as result + FROM Fixtures\\MartinGeorgiev\\Doctrine\\Entity\\ContainsArrays t + WHERE t.id = 1"; + $result = $this->executeDqlQuery($dql); + $this->assertTrue($result[0]['result']); + }
52-71: Add an empty-array case for completeness.PostgreSQL array && with an empty array should yield FALSE; adding a literal empty-array test tightens semantics.
+ #[Test] + public function returns_false_when_overlapping_with_empty_array_literal(): void + { + $dql = "SELECT OVERLAPS(t.textArray, ARRAY[]::text[]) as result + FROM Fixtures\\MartinGeorgiev\\Doctrine\\Entity\\ContainsArrays t + WHERE t.id = 1"; + $result = $this->executeDqlQuery($dql); + $this->assertFalse($result[0]['result']); + }
85-119: Use explicit JOIN for clarity (optional).The comma join works, but an explicit JOIN reads clearer in tests.
- $dql = 'SELECT OVERLAPS(t1.textArray, t2.textArray) as result - FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsArrays t1, - Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsArrays t2 - WHERE t1.id = 1 AND t2.id = 2'; + $dql = 'SELECT OVERLAPS(t1.textArray, t2.textArray) as result + FROM Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsArrays t1 + JOIN Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsArrays t2 WITH t2.id = 2 + WHERE t1.id = 1';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
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.659Z
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.
📚 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/OverlapsTest.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.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/OverlapsTest.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php
📚 Learning: 2025-08-26T22:06:36.659Z
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.659Z
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:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.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/Overlaps.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.phptests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.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/Overlaps.php
📚 Learning: 2025-08-24T23:39:18.782Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:7-7
Timestamp: 2025-08-24T23:39:18.782Z
Learning: This repository follows a concrete value object pattern without interfaces. All value objects (Point, DateRange, Int4Range, NumericRange, etc.) are concrete classes that directly implement required interfaces like \Stringable and \JsonSerializable. The codebase uses abstract base classes for shared functionality rather than interfaces for abstraction. Adding interfaces for value objects violates the established architectural patterns.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php
🧬 Code graph analysis (3)
tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php (1)
Overlaps(18-26)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (2)
OverlapsTest(11-37)getStringFunctions(13-18)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayTestCase.php (1)
ArrayTestCase(9-51)tests/Integration/MartinGeorgiev/TestCase.php (1)
executeDqlQuery(284-298)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (3)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php (1)
ORM(10-24)fixtures/MartinGeorgiev/Doctrine/Entity/ContainsArrays.php (1)
ORM(9-26)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php (1)
Overlaps(18-26)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Overlaps.php (1)
Overlaps(18-26)
⏰ 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). (16)
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + 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 17 + PostGIS 3.4 + PHP 8.3
- 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.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.4 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (4)
tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/RangeTypeTestCase.php (1)
10-10: Import update LGTM; registration remains consistent.No further changes needed here.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (2)
5-5: Unit test namespace realignment is correct.
9-9: Use statement updated to new FQCN — good.tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/OverlapsTest.php (1)
12-17: Function registration is correct and minimal.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Refactor
Tests
Chores