update query terminology #671
Conversation
* distance => distanceEqual * notDistance => distanceNotEqual
WalkthroughRenames spatial distance query types and factory methods from distance/notDistance to distanceEqual/distanceNotEqual across Query, adapters (MariaDB/Postgres), and validators. Updates tests to use the new API. Core binding and SQL generation logic remain the same; only type constants and method names changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client Code
participant Q as Query
participant V as Validator
participant A as Adapter (MariaDB/Postgres)
participant DB as Database
C->>Q: distanceEqual(attribute, [WKT, distance])
Q-->>C: Query object (type=TYPE_DISTANCE_EQUAL)
C->>V: validate(Query)
V-->>C: ok (method=FILTER, distanceEqual)
C->>A: build SQL + bindings(Query)
alt MariaDB
A->>A: ST_Distance(geom, ST_GeomFromText(:p0)) = :p1
else Postgres
A->>A: ST_DWithin(geom, ST_GeomFromText(:p0), :p1)
end
A->>DB: Execute with bound params
DB-->>A: Result set
A-->>C: Rows
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/Adapter/Scopes/SpatialTests.php (1)
1335-1344: Avoid flaky “ordering by distance” assertion; there’s no explicit order-by-distance.The test comments refer to ordering, but the query only filters by distance and applies a limit; no distance-based ordering is requested. Relying on result index can be brittle across adapters/implementations.
Prefer asserting membership rather than position. Suggested change:
- // Test ordering by distanceEqual from a specific point + // Test filtering by distance from a specific point $orderedByDistance = $database->find($collectionName, [ Query::distanceLessThan('location', [[[40.7829, -73.9654], 0.01]]), // Within ~1km Query::limit(10) ], Database::PERMISSION_READ); - $this->assertNotEmpty($orderedByDistance); - // First result should be closest to the reference point - $this->assertEquals('park1', $orderedByDistance[0]->getId()); + $this->assertNotEmpty($orderedByDistance); + // Ensure park1 is included in the results of the radius filter + $found = false; + foreach ($orderedByDistance as $doc) { + if ($doc->getId() === 'park1') { + $found = true; + break; + } + } + $this->assertTrue($found);
🧹 Nitpick comments (9)
src/Database/Adapter/Postgres.php (1)
1814-1825: Confirm intent: distanceEqual/NotEqual map to DWithin/NOT DWithin (<= / >), which differs from MariaDB’s = / !=Renaming to “distanceEqual” while using ST_DWithin implies “within radius” (<=) semantics, not strict equality. This diverges from MariaDB where distanceEqual currently uses ST_Distance = value. Please confirm the intended cross-adapter behavior:
- If the intent is “within/outside radius,” consider updating MariaDB to use <= and > (see MariaDB comment).
- If the intent is strict equality, Postgres should use ST_Distance =/!= for parity.
Right now, the adapters will return different result sets for the same query. I recommend aligning semantics across adapters to avoid subtle bugs in multi-db deployments.
If you decide to align to strict equality on Postgres, here’s the minimal change:
- return "ST_DWithin({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0), :{$placeholder}_1)"; + return "ST_Distance({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0)) = :{$placeholder}_1"; ... - return "NOT ST_DWithin({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0), :{$placeholder}_1)"; + return "ST_Distance({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0)) != :{$placeholder}_1";src/Database/Adapter/MariaDB.php (1)
1718-1729: Consider aligning distanceEqual/NotEqual semantics with Postgres (“within/outside radius”)Postgres uses ST_DWithin/NOT ST_DWithin (<= / >). MariaDB currently uses strict equality/inequality against ST_Distance, which won’t match Postgres behavior and is rarely useful in practice. If your intent is “within/outside radius” semantics, update MariaDB to mirror Postgres:
- return "ST_Distance({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0)) = :{$placeholder}_1"; + return "ST_Distance({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0)) <= :{$placeholder}_1"; ... - return "ST_Distance({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0)) != :{$placeholder}_1"; + return "ST_Distance({$alias}.{$attribute}, ST_GeomFromText(:{$placeholder}_0)) > :{$placeholder}_1";If the intent is truly “equality,” please adjust Postgres instead (see the Postgres comment) so both adapters agree.
src/Database/Validator/Query/Filter.php (1)
262-271: Tighten validation: ensure the distance argument is numericThe current check enforces the shape [[geometry, distance]] but doesn’t validate that the second element is numeric. Invalid types will bubble to SQL errors. Add an explicit numeric check to fail fast with a clear message.
- case Query::TYPE_DISTANCE_GREATER_THAN: - case Query::TYPE_DISTANCE_LESS_THAN: - if (count($value->getValues()) !== 1 || !is_array($value->getValues()[0]) || count($value->getValues()[0]) !== 2) { - $this->message = 'Distance query requires [[geometry, distance]] parameters'; - return false; - } - return $this->isValidAttributeAndValues($attribute, $value->getValues(), $method); + case Query::TYPE_DISTANCE_GREATER_THAN: + case Query::TYPE_DISTANCE_LESS_THAN: + if (count($value->getValues()) !== 1 || !is_array($value->getValues()[0]) || count($value->getValues()[0]) !== 2) { + $this->message = 'Distance query requires [[geometry, distance]] parameters'; + return false; + } + $params = $value->getValues()[0]; + if (!is_numeric($params[1])) { + $this->message = 'Distance parameter must be numeric'; + return false; + } + return $this->isValidAttributeAndValues($attribute, $value->getValues(), $method);src/Database/Query.php (1)
905-920: Add source-level BC aliases for Query::distance / Query::notDistance (optional)Renaming public builders is a breaking change for SDKs and user code. If a breaking release isn’t intended, provide soft aliases that delegate to the new methods. This preserves source compatibility while deprecating the old names.
public static function distanceEqual(string $attribute, array $values): self { return new self(self::TYPE_DISTANCE_EQUAL, $attribute, $values); } /** * Helper method to create Query with distanceNotEqual method * * @param string $attribute * @param array<mixed> $values * @return Query */ public static function distanceNotEqual(string $attribute, array $values): self { return new self(self::TYPE_DISTANCE_NOT_EQUAL, $attribute, $values); } + + /** + * @deprecated Use distanceEqual(...) instead. + */ + public static function distance(string $attribute, array $values): self + { + return self::distanceEqual($attribute, $values); + } + + /** + * @deprecated Use distanceNotEqual(...) instead. + */ + public static function notDistance(string $attribute, array $values): self + { + return self::distanceNotEqual($attribute, $values); + }Additionally, if you need wire-level compatibility for incoming JSON queries that still use "distance"/"notDistance", add a lightweight mapping in parseQuery before validating the method:
// inside parseQuery(), right after extracting $method (before isMethod($method) check) $method = match ($method) { 'distance' => self::TYPE_DISTANCE_EQUAL, 'notDistance' => self::TYPE_DISTANCE_NOT_EQUAL, default => $method, };This allows old payloads to continue working while your adapters switch on the new constants.
tests/e2e/Adapter/Scopes/SpatialTests.php (5)
601-606: Fix stale comment: code uses distanceLessThan, not distanceEqual.The comment contradicts the query call directly below.
Apply this minimal diff to align the comment with the code:
- // Spatial query on "drivers" using point distanceEqual + // Spatial query on "drivers" using point distanceLessThan
1159-1172: Update comments to match the actual operators used (distanceLessThan).Two comments say “distanceEqual” but both queries are distanceLessThan.
- // Test distanceEqual queries between shapes + // Test distanceLessThan queries between shapes ... - // Test distanceEqual queries to find nearby shapes + // Test distanceLessThan queries to find nearby shapes
1699-1708: COUNT/SUM comments mismatch the operator (should be distanceLessThan).Minor doc drift—align comments with the actual filter used.
- // COUNT with spatial distanceEqual filter + // COUNT with spatial distanceLessThan filter ... - // SUM with spatial distanceEqual filter + // SUM with spatial distanceLessThan filter
150-152: Standardize class name casing: use Query::..., not query::...PHP allows case-insensitive class references, but PSR-12 and typical PSR-4 autoloading conventions prefer correct casing for readability and tooling.
- 'equals' => query::equal('lineAttr', [[[1.0, 2.0], [3.0, 4.0]]]), // Exact same linestring - 'notEquals' => query::notEqual('lineAttr', [[[5.0, 6.0], [7.0, 8.0]]]), // Different linestring + 'equals' => Query::equal('lineAttr', [[[1.0, 2.0], [3.0, 4.0]]]), // Exact same linestring + 'notEquals' => Query::notEqual('lineAttr', [[[5.0, 6.0], [7.0, 8.0]]]), // Different linestring ... - 'equals' => query::equal('polyAttr', [[[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0], [0.0, 0.0]]]]), // Exact same polygon - 'notEquals' => query::notEqual('polyAttr', [[[[20.0, 20.0], [20.0, 30.0], [30.0, 30.0], [20.0, 20.0]]]]), // Different polygon + 'equals' => Query::equal('polyAttr', [[[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0], [0.0, 0.0]]]]), // Exact same polygon + 'notEquals' => Query::notEqual('polyAttr', [[[[20.0, 20.0], [20.0, 30.0], [30.0, 30.0], [20.0, 20.0]]]]), // Different polygon ... - query::notEqual('square', [[[0, 0], [0, 10], [10, 10], [10, 0], [0, 0]]]) // Different square + Query::notEqual('square', [[[0, 0], [0, 10], [10, 10], [10, 0], [0, 0]]]) // Different squareAlso applies to: 171-172, 1018-1019
1649-1655: Typo: rename testSptialAggregation to testSpatialAggregation.For clarity and grep-ability.
- public function testSptialAggregation(): void + public function testSpatialAggregation(): 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)
src/Database/Adapter/MariaDB.php(1 hunks)src/Database/Adapter/Postgres.php(1 hunks)src/Database/Query.php(5 hunks)src/Database/Validator/Queries.php(1 hunks)src/Database/Validator/Query/Filter.php(1 hunks)tests/e2e/Adapter/Scopes/SpatialTests.php(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/Validator/Query/Filter.phpsrc/Database/Query.phptests/e2e/Adapter/Scopes/SpatialTests.php
🧬 Code graph analysis (5)
src/Database/Adapter/Postgres.php (2)
src/Database/Query.php (2)
Query(8-1041)getValues(164-167)src/Database/Adapter/SQL.php (1)
convertArrayToWKT(2262-2301)
src/Database/Adapter/MariaDB.php (2)
src/Database/Query.php (1)
Query(8-1041)src/Database/Adapter/SQL.php (1)
convertArrayToWKT(2262-2301)
src/Database/Validator/Queries.php (1)
src/Database/Query.php (1)
Query(8-1041)
src/Database/Validator/Query/Filter.php (1)
src/Database/Query.php (1)
Query(8-1041)
tests/e2e/Adapter/Scopes/SpatialTests.php (1)
src/Database/Query.php (4)
Query(8-1041)distanceEqual(905-908)distanceNotEqual(917-920)distanceLessThan(941-944)
⏰ 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 (9)
src/Database/Validator/Queries.php (1)
111-113: LGTM: mappings updated to new distanceEqual/distanceNotEqual constantsThe method-to-type mapping is consistent with the renamed constants and keeps these under METHOD_TYPE_FILTER.
src/Database/Query.php (2)
33-35: LGTM: constants and isMethod/isSpatialQuery reflect the renameThe new TYPE_DISTANCE_EQUAL/TYPE_DISTANCE_NOT_EQUAL constants are declared and wired into isMethod/isSpatialQuery/TYPES. Looks consistent.
Also applies to: 83-85, 264-266, 288-290
905-920: No lingering legacy names detectedI ran the provided grep commands against the entire repository (excluding
vendor/) and found zero occurrences of the olddistance/notDistancemethods or related constants in PHP code, JSON fixtures, or documentation. All references have been successfully updated.tests/e2e/Adapter/Scopes/SpatialTests.php (6)
134-136: Renamed API usage looks correct (distanceEqual/distanceNotEqual) with proper value shape.The new operators are used with the expected [[[geometry], distance]] structure, consistent with prior expectations. Good boundary pick with sqrt(2) for an exact-on-threshold include.
428-437: Good boundary test for equality semantics at threshold 0.Using 0.0 cleanly demonstrates distanceEqual (<=) includes exact matches and distanceNotEqual (>) excludes them.
533-542: Repeat of boundary check for relationships is solid.Same semantics applied in Many-to-One scenario; reads well and should be stable across adapters.
638-647: Zero-threshold equality checks read clearly.Nice, concise validation that equality and non-equality behave as intended for points at the same coordinate.
1208-1218: Clear showcase of distanceEqual vs distanceNotEqual on circle_center.Nice use of 0.0 to make the boundary behavior explicit and adapter-agnostic.
10-11: No deprecated Query::distance()/notDistance() usages foundAll checks confirm the rename was applied cleanly across the repo:
- Repository-wide search (
rg -nP '\bQuery::\s*(distance|notDistance)\s*\(') returned no deprecated calls.- Only the new spatial equality operators (
Query::distanceEqualandQuery::distanceNotEqual) are present (e.g. in tests/e2e/Adapter/Scopes/SpatialTests.php).- Sanity-check for argument shapes flagged no mis-shaped calls.
No further updates needed.
2 participants
Summary by CodeRabbit
Refactor
Tests