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

Make test suite compliant with PostGIS #43

Open
andrewbt opened this issue Feb 28, 2017 · 3 comments
Open

Make test suite compliant with PostGIS #43

andrewbt opened this issue Feb 28, 2017 · 3 comments

Comments

@andrewbt
Copy link
Contributor

This is just notes of the issues I saw for now, I will clean this up later / split into separate issues:

http://postgis.net/docs/ST_NPoints.html (wrong function)
https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L122

http://postgis.net/docs/ST_Distance.html (cast to geography)
https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L130

https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L143 (could be replaced by the star variable above?)

https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L154 (should this be "location" and not the_geom??)

http://postgis.net/docs/ST_Intersects.html (the WKT needs to be casted to geometry)
https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L274

http://postgis.net/docs/ST_MakeEnvelope.html (need to swap order here, socrata within_box is different ordering of xmin, ymax, xmax, ymin than what postgis expects)
https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L290

http://postgis.net/docs/ST_Within.html (the WKT needs casting to geometry maybe? or use st_geomfromtext?)
https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L306

https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L314 (missing - but you could easily do carto's updated_at maybe?)

https://github.com/CityOfPhiladelphia/soda-carto/blob/master/test/index.js#L394 (missing?)

andrewbt added a commit that referenced this issue Mar 29, 2017
@andrewbt
Copy link
Contributor Author

Upon further research, going to need to revise some of these recommendations. Would be better to use the_geom_webmercator and some trigonometry for distance calculations and so on, rather than casting to ::geography. Will allow use of meters units, while also engaging the default geometry index on that column for better performance, while also being the most accurate considering our data is local to a city and not global.

(as described in https://carto.com/blog/nearest-neighbor-joins)

@timwis
Copy link
Contributor

timwis commented Apr 7, 2017

Bugs identified by this issue:

  • $select=num_points(location) L122
  • $select=distance_in_meters(location, ....) L130
  • $where=intersects(location, ....) L274
  • $where=within_box() L290
  • $where=within_polygon() L306

@timwis
Copy link
Contributor

timwis commented Apr 7, 2017

@andrewbt regarding L306, it looks like it does use GeometryFromText. So are we good on that one?

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

No branches or pull requests

2 participants