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

DATAMONGO-1127 - Add support for geoNear queries with distance information. #261

Closed
wants to merge 4 commits into from

Conversation

thomasdarimont
Copy link

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

thomasdarimont pushed a commit that referenced this pull request Dec 24, 2014
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

Original pull request: #261.
thomasdarimont pushed a commit that referenced this pull request Dec 29, 2014
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

Original pull request: #261.
thomasdarimont pushed a commit that referenced this pull request Dec 29, 2014
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

Original pull request: #261.
@odrotbohm
Copy link
Member

I don't quite like the addition of distanceField to NearQuery, the reason being that it's not applicable to general usage of NearQuery. As it basically describes which field the distance should be returned in in aggregation scenarios, what do you think of rather adding it to the GeoNearOperations type (either as constructor parameter if it's required or as additional builder method).

Thomas Darimont added 2 commits January 20, 2015 13:09
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

Original pull request: #261.
thomasdarimont pushed a commit that referenced this pull request Jan 20, 2015
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation.

Original pull request: #261.
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation.

Original pull request: #261.
* @return
* @since 1.7
*/
public static GeoNearOperation geoNear(NearQuery query, String distanceField) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we actually add an overload without the distance field applied so that basically people can mimic what they got when using new GeoNearOperation(nearQuery) before?

Copy link
Author

Choose a reason for hiding this comment

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

the geoNear(...) method was not exposed before this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I see that. Still, we need to keep the constructor in GeoNearOperations which then raises the question why geoNear(query, distance) is available as factory method but geoNear(query) is not. We should stay symmetric here.

Copy link
Author

Choose a reason for hiding this comment

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

Ack.

…ation.

Revised the code according to code review.
Note that the $geoNear Operation within an Aggregation requires a distanceField to be configured, therefore we make sure that it is set accordingly.
Added hints to use the static factory methods of Aggregation to the javadoc of the AggreationOperation classes.
thomasdarimont pushed a commit that referenced this pull request Jan 20, 2015
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

Original pull request: #261.
thomasdarimont pushed a commit that referenced this pull request Jan 20, 2015
…ation.

We now support geoNear queries in Aggregations. Exposed GeoNearOperation factory method in Aggregation. Introduced new distanceField property to NearQuery since it is required for geoNear queries in Aggregations.

Original pull request: #261.
@odrotbohm odrotbohm closed this Jan 20, 2015
@odrotbohm odrotbohm deleted the issue/DATAMONGO-1127 branch January 20, 2015 17:21
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.

None yet

2 participants