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 nearSphere queries when sorting by another key on Postgres #6026

Conversation

jaypatel24
Copy link

Currently the Postgres adapter always adds a sort on a GeoPoint field if doing a nearSphere query, which isn't the expected behavior.

Previously the added test case was passing for Mongo but failing for Postgres, but now it passes for both.

@dplewis
Copy link
Member

dplewis commented Sep 5, 2019

You can pass in a sorted parameter false.

parse-community/Parse-SDK-JS#582

query.withinKilometers('location', point, 4800.0, false);

@jaypatel24
Copy link
Author

Ah... I need to investigate further but it seems like the PostgresStorageAdapter might be ignoring that parameter, since it always adds the sort? If that's true, should we update it to only add the sort if sorted=true?

However, looking at these tests, it doesn't seem like sorted=true sets any additional fields in the query:
https://github.com/mtrezza/Parse-SDK-JS/blob/c594faaca9a17dc82edf7adc255e1aab46568a17/src/__tests__/ParseQuery-test.js#L604

@dplewis
Copy link
Member

dplewis commented Sep 5, 2019

https://github.com/parse-community/Parse-SDK-JS/blob/master/src/ParseQuery.js#L1304

We try to make Mongo and Postgres to have similar functionality.

https://docs.mongodb.com/manual/reference/operator/query/nearSphere/#sort-operation

If we merge your PR the following will happen.

Mongo will sort by default
Postgres will no longer sort by default

If sort is false, we use $centersphere which doesn't sort that way both db will have same functionality.

@jaypatel24
Copy link
Author

That makes sense, I think we can close this!

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.

2 participants