Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rewrite of near & nearSphere count queries using geoJson to geoWithin. #4006

Closed
wants to merge 4 commits into from

Conversation

christophstrobl
Copy link
Member

$near and $nearSphere queries are not supported via countDocuments and the used aggregation match stage and need to be rewritten to $geoWithin. The existing logic did not cover usage of geoJson types, which is fixed now. In case of nearSphere it is also required to convert the $maxDistance argument (given in meters for geoJson) to radians which is used by $geoWithin $centerSphere.

Resolves: #4004

$near and $nearSphere queries are not supported via countDocuments and the used aggregation match stage and need to be rewritten to $geoWithin. The existing logic did not cover usage of geoJson types, which is fixed now. In case of nearSphere it is also required to convert the $maxDistance argument (given in meters for geoJson) to radians which is used by $geoWithin $centerSphere.

Related to #2925
Update Javadoc to mention unit of measure for min/maxDistance depending on usage of geoJson.
Also remove unused imports from tests
@@ -193,6 +196,24 @@ private static Document createGeoWithin(String key, Document source, @Nullable O
return new Document("$and", criteria);
}

private static Number getMaxDistance(Document source, Object $near, boolean spheric) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have MetricConversion for some conversion calculations. It would make sense to reuse those bits.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some, not all. Still we could move those bits there 👍

@mp911de mp911de self-assigned this Apr 19, 2022
@mp911de mp911de added this to the 3.4.4 (2021.2.4) milestone Sep 21, 2022
mp911de pushed a commit that referenced this pull request Sep 21, 2022
$near and $nearSphere queries are not supported via countDocuments and the used aggregation match stage and need to be rewritten to $geoWithin. The existing logic did not cover usage of geoJson types, which is fixed now. In case of nearSphere it is also required to convert the $maxDistance argument (given in meters for geoJson) to radians which is used by $geoWithin $centerSphere.

Closes #4004
Original pull request: #4006.
Related to #2925
mp911de pushed a commit that referenced this pull request Sep 21, 2022
Update Javadoc to mention unit of measure for min/maxDistance depending on usage of geoJson.
Also remove unused imports from tests

See #4004
Original pull request: #4006.
mp911de added a commit that referenced this pull request Sep 21, 2022
Reformat code.

See #4004
Original pull request: #4006.
mp911de pushed a commit that referenced this pull request Sep 21, 2022
$near and $nearSphere queries are not supported via countDocuments and the used aggregation match stage and need to be rewritten to $geoWithin. The existing logic did not cover usage of geoJson types, which is fixed now. In case of nearSphere it is also required to convert the $maxDistance argument (given in meters for geoJson) to radians which is used by $geoWithin $centerSphere.

Closes #4004
Original pull request: #4006.
Related to #2925
mp911de pushed a commit that referenced this pull request Sep 21, 2022
Update Javadoc to mention unit of measure for min/maxDistance depending on usage of geoJson.
Also remove unused imports from tests

See #4004
Original pull request: #4006.
mp911de added a commit that referenced this pull request Sep 21, 2022
Reformat code.

See #4004
Original pull request: #4006.
@mp911de
Copy link
Member

mp911de commented Sep 21, 2022

That's merged, polished, and backported now.

@mp911de mp911de closed this Sep 21, 2022
@mp911de mp911de deleted the issue/4004 branch September 21, 2022 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Point must only contain numeric elements when using near count query with GeoJson points
2 participants