updated distance query handling #673
Conversation
make it more cleaner by introducing a third parameter distance for clean interface
WalkthroughAdds an explicit distance parameter to distance-based spatial query helpers in Query, changes how values are encoded to pair coordinates with distance, updates isMethod/isSpatialQuery checks, and refactors E2E spatial tests to call the new signatures across many geometry types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test / Caller
participant Q as Query::distanceXxx
participant A as Adapter / Engine
Note over T,Q: New API: distanceXxx(attribute, coords, distance)
T->>Q: distanceEqual(attr, coords, distance)
activate Q
Q->>Q: Build payload: values = [[coords, distance]]\nSet operator = DISTANCE_EQUAL
Q-->>T: Query instance
deactivate Q
T->>A: execute(query)
activate A
A->>A: Parse operator and [[coords, distance]]\nApply spatial distance filter and ordering
A-->>T: Results
deactivate A
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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: 1
🧹 Nitpick comments (10)
src/Database/Query.php (6)
908-911: Distance query helper: preserve wire shape, add tiny style/defensive tweaksRecommend spacing after comma to match project style and a light non-negative guard on $distance.
Apply:
- public static function distanceEqual(string $attribute, array $values, int|float $distance): self - { - return new self(self::TYPE_DISTANCE_EQUAL, $attribute, [[$values,$distance]]); - } + public static function distanceEqual(string $attribute, array $values, int|float $distance): self + { + // Optional: distance should be non-negative + if ($distance < 0) { + throw new QueryException('Distance must be non-negative'); + } + return new self(self::TYPE_DISTANCE_EQUAL, $attribute, [[$values, $distance]]); + }
921-924: DistanceNotEqual(): mirror spacing/guard for consistencyKeep helpers consistent with a non-negative guard and spacing.
- public static function distanceNotEqual(string $attribute, array $values, int|float $distance): self - { - return new self(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, [[$values,$distance]]); - } + public static function distanceNotEqual(string $attribute, array $values, int|float $distance): self + { + if ($distance < 0) { + throw new QueryException('Distance must be non-negative'); + } + return new self(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, [[$values, $distance]]); + }
934-937: DistanceGreaterThan(): minor style/guard parityAlign with other helpers.
- public static function distanceGreaterThan(string $attribute, array $values, int|float $distance): self - { - return new self(self::TYPE_DISTANCE_GREATER_THAN, $attribute, [[$values,$distance]]); - } + public static function distanceGreaterThan(string $attribute, array $values, int|float $distance): self + { + if ($distance < 0) { + throw new QueryException('Distance must be non-negative'); + } + return new self(self::TYPE_DISTANCE_GREATER_THAN, $attribute, [[$values, $distance]]); + }
942-946: Docblock: add @return for distanceLessThanReturn type annotation is missing here (present in the other three).
/** * Helper method to create Query with distanceLessThan method * * @param string $attribute * @param array<mixed> $values * @param int|float $distance + * @return Query */
946-949: DistanceLessThan(): style/guard consistencyMirror spacing and optional validation.
- public static function distanceLessThan(string $attribute, array $values, int|float $distance): self - { - return new self(self::TYPE_DISTANCE_LESS_THAN, $attribute, [[$values,$distance]]); - } + public static function distanceLessThan(string $attribute, array $values, int|float $distance): self + { + if ($distance < 0) { + throw new QueryException('Distance must be non-negative'); + } + return new self(self::TYPE_DISTANCE_LESS_THAN, $attribute, [[$values, $distance]]); + }
900-949: DRY the four distance helpers (optional)All four helpers differ only by TYPE_*. Consider a tiny private factory to remove duplication and centralize validation.
Example (add anywhere in the class, e.g., near other helpers):
/** @internal */ private static function makeDistance(string $type, string $attribute, array $values, int|float $distance): self { if ($distance < 0) { throw new QueryException('Distance must be non-negative'); } return new self($type, $attribute, [[$values, $distance]]); }Then:
public static function distanceEqual(string $attribute, array $values, int|float $distance): self { return self::makeDistance(self::TYPE_DISTANCE_EQUAL, $attribute, $values, $distance); }tests/e2e/Adapter/Scopes/SpatialTests.php (4)
134-136: Point distance helpers: good API usage; prefer computed sqrt(2) for clarityThe switch to Query::distanceXxx(attr, coords, distance) looks correct. Minor readability nit: compute sqrt(2) instead of hardcoding full precision.
- 'distanceEqual' => Query::distanceEqual('pointAttr', [5.0, 5.0], 1.4142135623730951), + 'distanceEqual' => Query::distanceEqual('pointAttr', [5.0, 5.0], sqrt(2)),
532-533: One-to-many: thresholds exercise near/far cases wellNice gradient of thresholds for distanceGreaterThan/LessThan. Consider small epsilons when asserting strict boundaries on some adapters to avoid float edge flakes.
If you’ve seen flakiness, try nudging 0.05/0.1 thresholds by +/−1e-6.
Also applies to: 538-539, 545-546, 551-552, 557-558, 563-564, 569-570
1328-1331: Combinations: logical AND/OR with distance filters looks correct; ordering caveatCombining distance filters with AND/OR is a strong addition. For the “ordered by distance” case, double-check whether adapters guarantee sorting by distance; if not, assert membership or add explicit ordering if available.
Would you like a follow-up patch that relaxes the ordering assertion to verify set membership unless an adapter explicitly advertises distance ordering?
Also applies to: 1340-1341, 1347-1348, 1353-1355, 1360-1362, 1366-1368, 1372-1374, 1378-1381, 1388-1391
1328-1331: Add a parse/toString round-trip for new distance queries (optional)To guard the public JSON shape, add a small test that builds a JSON query with ["values" => [ [coords, distance] ]] and ensures Query::parse(...)->toString() round-trips to the same structure for each distance operator.
I can add a parametrized data provider across distanceEqual/NotEqual/LessThan/GreaterThan if you want.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/Database/Query.php(2 hunks)tests/e2e/Adapter/Scopes/SpatialTests.php(15 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#661
File: tests/e2e/Adapter/Scopes/SpatialTests.php:180-186
Timestamp: 2025-08-14T06:35:30.429Z
Learning: Query::distance method in Utopia\Database\Query expects an array of values parameter, where each value is a [geometry, distance] pair. So the correct format is Query::distance('attribute', [[[lat, lng], distance]]) where the outer array contains the values and each value is [geometry, distance].
📚 Learning: 2025-08-14T06:35:30.429Z
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#661
File: tests/e2e/Adapter/Scopes/SpatialTests.php:180-186
Timestamp: 2025-08-14T06:35:30.429Z
Learning: Query::distance method in Utopia\Database\Query expects an array of values parameter, where each value is a [geometry, distance] pair. So the correct format is Query::distance('attribute', [[[lat, lng], distance]]) where the outer array contains the values and each value is [geometry, distance].
Applied to files:
src/Database/Query.phptests/e2e/Adapter/Scopes/SpatialTests.php
🧬 Code graph analysis (1)
tests/e2e/Adapter/Scopes/SpatialTests.php (1)
src/Database/Query.php (5)
Query(8-1046)distanceEqual(908-911)distanceNotEqual(921-924)distanceLessThan(946-949)distanceGreaterThan(934-937)
⏰ 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). (1)
- GitHub Check: Setup & Build Docker Image
🔇 Additional comments (8)
src/Database/Query.php (1)
266-268: isMethod(): new distance operators correctly whitelistedBoth TYPE_DISTANCE_GREATER_THAN and TYPE_DISTANCE_LESS_THAN are now accepted. Matches constants/TYPES usage elsewhere.
tests/e2e/Adapter/Scopes/SpatialTests.php (7)
165-177: Linestring distance queries: coverage looks solidEqual/NotEqual and both inequality directions are exercised against matching and non-matching linestrings. Nice coverage.
200-212: Polygon distance queries: consistent with new signatureThe distance tests for polygon rings mirror the linestring set. LGTM.
286-287: One-to-one relationship: distanceLessThan migration is correctQueries now pass coordinates and distance separately. Semantics remain clear and adapter-gated by spatial support. All good.
Also applies to: 300-301
631-632: Many-to-many: consistent migration and edge assertionsUsing 0.0 and tiny thresholds to validate equality vs inequality is effective. No issues spotted.
Also applies to: 637-639, 643-645, 650-652, 656-658, 662-664, 668-670, 674-676
1189-1191: Complex shapes: distance filters applied across point/line/polygonGood spread of near/far filters and equality checks for multiple geometry types. Note: assumptions about implicit ordering by distance in later tests may be adapter-specific (see below).
If any adapter doesn’t sort by distance within a distanceLessThan filter, avoid asserting on the first element’s ID for “closest” unless ordering is explicitly guaranteed.
Also applies to: 1196-1198, 1203-1207, 1210-1212, 1218-1222, 1225-1229, 1232-1236, 1239-1243, 1245-1249, 1251-1261
1629-1631: Bulk ops: distance filters validate post-bulk data wellDistance-based finds after createDocuments/createOrUpdate are a good smoke test across many records. LGTM.
Also applies to: 1636-1638, 1643-1645, 1650-1652, 1657-1659, 1664-1666
1742-1746: Aggregation + distance filters: nice COUNT/SUM coverageValidates that aggregation honors spatial distance constraints. Looks correct.
Also applies to: 1753-1755
| 'distanceEqual' => Query::distanceEqual('pointAttr', [5.0, 5.0], 1.4142135623730951), | ||
| 'distanceNotEqual' => Query::distanceNotEqual('pointAttr', [1.0, 1.0], 0.0), | ||
| 'intersects' => Query::intersects('pointAttr', [[6.0, 6.0]]), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repository-wide: search for old nested distance payload usage
Since the signature changed repo-wide, ensure no callers still pass the legacy [[[coords], dist]] at the call-site.
Run:
Also applies to: 165-177, 200-212, 286-287, 300-301, 532-533, 538-539, 545-546, 551-552, 557-558, 563-564, 569-570, 631-632, 637-639, 643-645, 650-652, 656-658, 662-664, 668-670, 674-676, 1189-1191, 1196-1198, 1203-1207, 1210-1212, 1218-1222, 1225-1229, 1232-1236, 1239-1243, 1245-1249, 1251-1261, 1328-1331, 1340-1341, 1347-1348, 1353-1355, 1360-1362, 1366-1368, 1372-1374, 1378-1381, 1388-1391, 1629-1631, 1636-1638, 1643-1645, 1650-1652, 1657-1659, 1664-1666, 1742-1746, 1753-1755
🏁 Script executed:
#!/bin/bash
# Find old-style nested payloads at call sites like distanceXxx('attr', [[[...]], dist?])
# Heuristic: look for distance* calls where the second arg starts with triple brackets.
rg -nP --type=php -C1 $'Query::distance(?:Equal|NotEqual|LessThan|GreaterThan)\\(\\s*[^,]+,\\s*\\[\\s*\\[\\s*\\['Length of output: 1341
Update legacy nested distance payload usage in SpatialTests
The following test call sites are still using the old nested-payload signature ([[[…]], dist]) and must be updated to the new signature ([…], dist):
-
tests/e2e/Adapter/Scopes/SpatialTests.php
• Lines 202–205:'distanceEqual' => Query::distanceEqual('polyAttr', [[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0], [0.0, 0.0]]], 0.0), 'distanceNotEqual' => Query::distanceNotEqual('polyAttr', [[[20.0, 20.0], [20.0, 30.0], [30.0, 30.0], [20.0, 20.0]]], 0.0), 'distanceLessThan' => Query::distanceLessThan('polyAttr', [[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0], [0.0, 0.0]]], 0.1), 'distanceGreaterThan'=> Query::distanceGreaterThan('polyAttr', [[[20.0, 20.0], [20.0, 30.0], [30.0, 30.0], [20.0, 20.0]]], 0.1),
• Update each to remove one level of nesting, e.g.:
Query::distanceEqual('polyAttr', [[0.0, 0.0], [0.0, 10.0], [10.0, 10.0], [0.0, 0.0]], 0.0)
-
tests/e2e/Adapter/Scopes/SpatialTests.php
• Line 1252:Query::distanceEqual('rectangle', [[[0, 0], [0, 10], [20, 10], [20, 0], [0, 0]]], 0.0)
• Should become:
Query::distanceEqual('rectangle', [[0, 0], [0, 10], [20, 10], [20, 0], [0, 0]], 0.0)
Please update these instances (and any others found) to use the flattened payload format to match the updated Query::distance* signature.
🤖 Prompt for AI Agents
In tests/e2e/Adapter/Scopes/SpatialTests.php (notably around lines 202–205 and
line 1252), the SpatialTests still call Query::distance* with an extra nesting
level for polygon/rectangle coordinates (using [[[...]]], dist) — update each to
remove one level of array nesting so the coordinate list is a single array of
points followed by the distance (e.g. change [[[x,y],[x,y],...]] to
[[x,y],[x,y],...]); scan the file for other
Query::distanceEqual/NotEqual/LessThan/GreaterThan calls and apply the same
one-level flattening to their payload arguments.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Database/Query.php (1)
910-910: About the nested pair shape [[$values, $distance]]Keeping the values shape as a 2D array aligns with the validator that expects each entry to be a [geometry, distance] pair. This matches prior guidance.
🧹 Nitpick comments (3)
src/Database/Query.php (3)
910-910: Style nit: add a space after the comma inside the inner pairConform to PSR-style spacing for arrays.
Apply:
- return new self(self::TYPE_DISTANCE_EQUAL, $attribute, [[$values,$distance]]); + return new self(self::TYPE_DISTANCE_EQUAL, $attribute, [[$values, $distance]]); - return new self(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, [[$values,$distance]]); + return new self(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, [[$values, $distance]]); - return new self(self::TYPE_DISTANCE_GREATER_THAN, $attribute, [[$values,$distance]]); + return new self(self::TYPE_DISTANCE_GREATER_THAN, $attribute, [[$values, $distance]]); - return new self(self::TYPE_DISTANCE_LESS_THAN, $attribute, [[$values,$distance]]); + return new self(self::TYPE_DISTANCE_LESS_THAN, $attribute, [[$values, $distance]]);Also applies to: 923-923, 936-936, 949-949
908-911: Deduplicate construction and guard invalid distances with a small helperFour nearly identical methods can delegate to a single constructor helper. This also centralizes validation (e.g., reject negative distances).
Apply within these methods:
- return new self(self::TYPE_DISTANCE_EQUAL, $attribute, [[$values, $distance]]); + return self::makeDistanceQuery(self::TYPE_DISTANCE_EQUAL, $attribute, $values, $distance); - return new self(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, [[$values, $distance]]); + return self::makeDistanceQuery(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, $values, $distance); - return new self(self::TYPE_DISTANCE_GREATER_THAN, $attribute, [[$values, $distance]]); + return self::makeDistanceQuery(self::TYPE_DISTANCE_GREATER_THAN, $attribute, $values, $distance); - return new self(self::TYPE_DISTANCE_LESS_THAN, $attribute, [[$values, $distance]]); + return self::makeDistanceQuery(self::TYPE_DISTANCE_LESS_THAN, $attribute, $values, $distance);Add this helper inside the class (e.g., near other helpers):
/** * @internal Centralizes distance query construction and validation * @param string $type One of the TYPE_DISTANCE_* constants * @param string $attribute * @param array<mixed> $values Geometry payload * @param int|float $distance Non-negative distance */ private static function makeDistanceQuery(string $type, string $attribute, array $values, int|float $distance): self { if ($distance < 0) { throw new \InvalidArgumentException('Distance must be non-negative.'); } // Keep the storage shape consistent with validator expectations: [ [ geometry, distance ] ] return new self($type, $attribute, [[$values, $distance]]); }Also applies to: 921-924, 934-937, 947-949
905-906: Doc clarity: specify geometry shape and distance unitsCurrent tags list the new parameter but omit usage details. Please clarify:
- What geometry shapes are accepted in $values for these helpers (e.g., Point as [lat, lng]; LineString as array<array{lat: float, lng: float}>, Polygon as array<array<array{...}>>)?
- What are the expected distance units (meters? same units as the adapter/SRID)?
Example doc refinement (adjust units/shapes to match adapter behavior):
- * @param array<mixed> $values - * @param int|float $distance + * @param array<mixed> $values Geometry payload. For Point use [lat, lng]. For LineString use [[lat, lng], ...]. For Polygon use [[[lat, lng], ...], ...]. + * @param int|float $distance Non-negative distance in meters (confirm units per adapter/SRID).Also applies to: 918-918, 931-931, 944-944
📜 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)
src/Database/Query.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#661
File: tests/e2e/Adapter/Scopes/SpatialTests.php:180-186
Timestamp: 2025-08-14T06:35:30.429Z
Learning: Query::distance method in Utopia\Database\Query expects an array of values parameter, where each value is a [geometry, distance] pair. So the correct format is Query::distance('attribute', [[[lat, lng], distance]]) where the outer array contains the values and each value is [geometry, distance].
📚 Learning: 2025-08-14T06:35:30.429Z
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#661
File: tests/e2e/Adapter/Scopes/SpatialTests.php:180-186
Timestamp: 2025-08-14T06:35:30.429Z
Learning: Query::distance method in Utopia\Database\Query expects an array of values parameter, where each value is a [geometry, distance] pair. So the correct format is Query::distance('attribute', [[[lat, lng], distance]]) where the outer array contains the values and each value is [geometry, distance].
Applied to files:
src/Database/Query.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Adapter Tests (Postgres)
- GitHub Check: Unit Test
🔇 Additional comments (3)
src/Database/Query.php (3)
266-267: Added distanceGreaterThan/LessThan to isMethod — LGTMThese additions ensure the new spatial distance comparators are recognized during parsing/validation.
908-908: Explicit $distance parameter on distanceEqual/NotEqual/GreaterThan/LessThan — API shape looks goodThe signatures are clear and consistent, improving call-site ergonomics over the prior nested-pair-only approach.
Also applies to: 921-921, 934-934, 947-947
908-911: All Query::distance call sites now include the $distance argument*
I ran the audit script and confirmed there are no remaining two-argument invocations of distanceEqual, distanceNotEqual, distanceLessThan, or distanceGreaterThan. Great work!
2 participants
make it more cleaner by introducing a third parameter distance for clean interface
Summary by CodeRabbit
New Features
Refactor
Tests