Skip to content

Fix GeoPoint issues on PostgreSQL (#3285 & #3659) #3660

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Fix GeoPoint issues on PostgreSQL (#3285 & #3659) #3660

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
05917b6
Fix GeoPoint issues on PostgreSQL (#3285 & #3659)
vferdiansyah Mar 23, 2017
651d785
Fix for failed tests
vferdiansyah Mar 27, 2017
6c8c494
Merge branch 'master' into master
flovilmart Apr 7, 2017
859fe26
Add a near location test case
vferdiansyah Apr 7, 2017
f18cc3c
Merge branch 'master' into master
vferdiansyah May 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ const buildWhereClause = ({ schema, query, index }) => {
const point = fieldValue.$nearSphere;
const distance = fieldValue.$maxDistance;
const distanceInKM = distance * 6371 * 1000;
patterns.push(`ST_distance_sphere($${index}:name::geometry, POINT($${index + 1}, $${index + 2})::geometry) <= $${index + 3}`);
sorts.push(`ST_distance_sphere($${index}:name::geometry, POINT($${index + 1}, $${index + 2})::geometry) ASC`)
patterns.push(`ST_distance_sphere($${index}:name::geometry, POINT($${index + 2}, $${index + 1})::geometry) <= $${index + 3}`);
sorts.push(`ST_distance_sphere($${index}:name::geometry, POINT($${index + 2}, $${index + 1})::geometry) ASC`)
values.push(fieldName, point.longitude, point.latitude, distanceInKM);
index += 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fine.
Although I think it might be more readable to change the order of latitude/longitude when pushing them in values.

So, leaving lines 328/329 as they originally were and replacing values.push(fieldName, point.longitude, point.latitude, distanceInKM); with values.push(fieldName, point.latitude, point.longitude, distanceInKM); on line 330

Copy link
Contributor

@vitaly-t vitaly-t Mar 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

whenever in doubt about what pg-promise outputs, you can always run it through pgp.as.format(query, values) locally, throw the result into the console and see what's there ;)

Copy link
Contributor

@vitaly-t vitaly-t Mar 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I may have incomplete understanding in what that ST_distance_sphere does, but just to make sure, the :name formatting results in the value being wrapped into double quotes, as in ST_distance_sphere("value", , as per the SQL Names formatting.

Are you guys sure this is what the function actually expects? 'Cos I'm not sure...

It looks very suspecious. Looks to me from the API documentation that it should be a string. Or am I wrong there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Original tests were passing, so I'd trust it. But I may be wrong

Copy link
Author

Choose a reason for hiding this comment

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

Solution suggested by @kulshekhar also works actually. Just tried it on my local machine.

screen shot 2017-03-24 at 16 37 51
Not sure if this is what you meant @vitaly-t but as far as I can see, the results aren't wrapped in double quotes at all.

Yes, original tests were passing @flovilmart and it works fine so far on my app.

Copy link
Contributor

@vitaly-t vitaly-t Mar 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The best way to see what is really going on in terms of the queries being executed, is with pg-monitor. You can diagnose issues much quicker than.

For that just update PosgresClient.js like this:

  1. Add var monitor = require('pg-monitor');
  2. Call monitor.attach(dbOptions); to attach to the pg options object
  3. Optional, for best look: monitor.setTheme('matrix');

And of course install it first npm install pg-monitor --save

Then you will be able to see every query the module executes 😉

}
Expand Down Expand Up @@ -1071,8 +1071,8 @@ export class PostgresStorageAdapter {
}
if (object[fieldName] && schema.fields[fieldName].type === 'GeoPoint') {
object[fieldName] = {
latitude: object[fieldName].y,
longitude: object[fieldName].x
latitude: object[fieldName].x,
longitude: object[fieldName].y
}
}
if (object[fieldName] && schema.fields[fieldName].type === 'File') {
Expand Down