Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Query items by geolocation and distance return incorrect results #266

Closed
cychiuae opened this issue Jan 18, 2017 · 6 comments
Closed

Query items by geolocation and distance return incorrect results #266

cychiuae opened this issue Jan 18, 2017 · 6 comments
Assignees
Labels

Comments

@cychiuae
Copy link

cychiuae commented Jan 18, 2017

Suppose we have a record type product

We first create a product with location in Tower Hamlet, UK (lat: 51.5153574, lng: -0.0525232)

const Product = skygear.Record.extend('product');
const product = new Product({
  name: 'product 1',
  price: 100,
  location: new skygear.Geolocation(51.5153574, -0.0525232),
});
skygear.publicDB.save(product);

Then, we query product which within 5km from Lai Chi Kwok (lat: 22.3364240917999, lng: 114.14808275214)

const query = new skygear.Query(Product);
query. distanceLessThan('location', new skygear.Geolocation(22.3364240917999, 114.14808275214), 5000);
skygear.publicDB.query(query).then((results) => {
   console.log(results);  // we can see product 1 can be queried
});
  • Skygear Server Date/Version: v0.20.0

Skygear uses ST_DWithin incorrectly.
Please look at http://postgis.net/docs/ST_DWithin.html

Expected Results

Far products should not be queried

Actual Results

Far products can be queried

@cheungpat
Copy link
Contributor

Skygear uses ST_DWithin incorrectly.

How? If you know the answer it would be great if you could state that.

@cheungpat
Copy link
Contributor

I think I know what you are hinting here. The third parameter is interpreted as degrees rather than meters.

@cychiuae
Copy link
Author

Not a PostgreSQL expert but I think there is a problem when passing Geometry point as Lat Lng to ST_DWithin.
Simple fix will be casting the GPoint to geography

@cheungpat
Copy link
Contributor

cheungpat commented Jan 19, 2017

@cychiuae Sounds good, let’s cast it to geography. You need to apply an index on the column using a geography type to make use of the index.

@royuen royuen assigned cheungpat and unassigned rickmak Jan 19, 2017
@louischan-oursky
Copy link
Contributor

http://postgis.net/docs/manual-2.3/using_postgis_dbmanagement.html#PostGIS_GeographyVSGeometry

Always casting it to geography incurs performance penalty. By the way, if we decide to cast it anyway, would it be better to store it as Geography in the first place?

TypeLocation = "geometry(Point)"

cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
@cheungpat cheungpat added In Review and removed Focus labels Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
@cheungpat
Copy link
Contributor

I am going to say the objective of this issue is to fix the bug that the distance predicate does not work.

According to http://gis.stackexchange.com/questions/221978/st-dwithin-calc-meters-transform-or-cast:

You need for your query and your index to harmonize. If you do not want to change the projection of your data, you will have to use a functional index, for example, if you create a function geography index, you can use a geography-based query:

CREATE INDEX mytable_geog_x 
  ON mytable USING GIST (geography(geom));

SELECT * 
  FROM mytable 
  WHERE ST_DWithin(geography(geom), %anothergeography, %radius);

Please open another issue suggesting that we switch to geography instead of geometry.

cheungpat added a commit to cheungpat/skygear-server that referenced this issue Jan 19, 2017
rickmak pushed a commit that referenced this issue Jan 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants