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

[5.5] Geo Spatial blueprint methods #21056

Merged
merged 1 commit into from Sep 7, 2017

Conversation

Projects
None yet
5 participants
@antonkomarev
Contributor

antonkomarev commented Sep 7, 2017

I've targeted it to 5.5 but if 5.6 (master) is more appropriate for it - I'm ready to make another one PR.

This PR introduces Geo spatial blueprint methods to MySQL, PostgreSQL, SQLite. It allows to create database engine agnostic spatial data types columns without using raw expressions and conditionals.

Schema::create('conferences', function (Blueprint $table) {
    $table->increments('id');
    $table->string('name');
    $table->point('location')->nullable();
    $table->timestamps();
});

Complete list of methods:

  • geometry (not supported by PostgreSQL)
  • point
  • lineString
  • polygon
  • geometryCollection
  • multiPoint
  • multiLineString
  • multiPolygon

If blueprints PR will be accepted I will continue integration of query\eloquent Geo spatial related methods. They are much more complicated and requires to implement all geometry classes. As for now I'm making it as external package. Current functionality already implemented in package too, but it's pretty common stuff and doesn't require any new dependencies.

Note: Latest stable MySQL and PostgreSQL has spatial data types support out of the box. SQLite has extended version called SpatiaLite.

*/
protected function typeGeometry(Fluent $column)
{
throw new \Exception('Geometry data type not supported for current database engine.');

This comment has been minimized.

@antonkomarev

antonkomarev Sep 7, 2017

Contributor

Didn't know what type of exception would be better to throw here.

@antonkomarev

antonkomarev Sep 7, 2017

Contributor

Didn't know what type of exception would be better to throw here.

This comment has been minimized.

@antonkomarev

antonkomarev Sep 7, 2017

Contributor

I've thought about RuntimeException too.

@antonkomarev

antonkomarev Sep 7, 2017

Contributor

I've thought about RuntimeException too.

@taylorotwell taylorotwell merged commit 8f92b9f into laravel:5.5 Sep 7, 2017

2 checks passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@antonkomarev antonkomarev deleted the cybercog:feature/geospatial-blueprint-methods branch Sep 7, 2017

@antonkomarev

This comment has been minimized.

Show comment
Hide comment
@antonkomarev

antonkomarev Sep 7, 2017

Contributor

Thank you! ❤️ This change removes about to 12 classes from the package.

Contributor

antonkomarev commented Sep 7, 2017

Thank you! ❤️ This change removes about to 12 classes from the package.

@edsamonte

This comment has been minimized.

Show comment
Hide comment
@edsamonte

edsamonte Sep 12, 2017

Contributor

Any Eloquent guide for this?

Contributor

edsamonte commented Sep 12, 2017

Any Eloquent guide for this?

@antonkomarev

This comment has been minimized.

Show comment
Hide comment
@antonkomarev

antonkomarev Sep 12, 2017

Contributor

@edsamonte this PR includes only blueprint methods to create spatial data columns. To add Spatial queries to Eloquent there are much more things should be solved. First of all implement all geometry classes, WKT & WKB format parsers and generators.
There will be requirement to add more dependencies to Illuminate\Database package and I'm not sure Taylor will accept all of them. I've got all of this things done in extra 3 packages, but they are not documented and private at this moment. I need more time to refactor them, cover with documentation, only then they will be ready to be published.

Contributor

antonkomarev commented Sep 12, 2017

@edsamonte this PR includes only blueprint methods to create spatial data columns. To add Spatial queries to Eloquent there are much more things should be solved. First of all implement all geometry classes, WKT & WKB format parsers and generators.
There will be requirement to add more dependencies to Illuminate\Database package and I'm not sure Taylor will accept all of them. I've got all of this things done in extra 3 packages, but they are not documented and private at this moment. I need more time to refactor them, cover with documentation, only then they will be ready to be published.

@denaje

This comment has been minimized.

Show comment
Hide comment
@denaje

denaje Jul 26, 2018

Contributor

Why do you say that the geometry type is "not supported by PostgreSQL"? If you have PostGIS (which I assume is required for all of these types), geometry is a perfectly valid column type. Instead of returning "geography({$type}, 4326)" you could return "geometry" or "geometry('geometry', 4326)" or even "geography('geometry', 4326)". Is there a reason this was not implemented?

Contributor

denaje commented Jul 26, 2018

Why do you say that the geometry type is "not supported by PostgreSQL"? If you have PostGIS (which I assume is required for all of these types), geometry is a perfectly valid column type. Instead of returning "geography({$type}, 4326)" you could return "geometry" or "geometry('geometry', 4326)" or even "geography('geometry', 4326)". Is there a reason this was not implemented?

@antonkomarev

This comment has been minimized.

Show comment
Hide comment
@antonkomarev

antonkomarev Jul 26, 2018

Contributor

@denaje thank you for raising this important question!

I excluded geometry type because it wasn't worked in my PostgreSQL and I didn't found any solution. If you could make it work - post a PR covered with tests and I'm sure Taylor will merge it.

Contributor

antonkomarev commented Jul 26, 2018

@denaje thank you for raising this important question!

I excluded geometry type because it wasn't worked in my PostgreSQL and I didn't found any solution. If you could make it work - post a PR covered with tests and I'm sure Taylor will merge it.

@paulofreitas

This comment has been minimized.

Show comment
Hide comment
@paulofreitas

paulofreitas Jul 27, 2018

Contributor

Indeed, this is an interesting question. Thanks @denaje! 👍

There are two different spatial types used in PostGIS: geometry (planar) and geography (spheroid). Quoting the PostGIS documentation:

The basis for the PostGIS geometry type is a plane. The shortest path between two points on the plane is a straight line. That means calculations on geometries (areas, distances, lengths, intersections, etc) can be calculated using cartesian mathematics and straight line vectors.

The basis for the PostGIS geographic type is a sphere. The shortest path between two points on the sphere is a great circle arc. That means that calculations on geographies (areas, distances, lengths, intersections, etc) must be calculated on the sphere, using more complicated mathematics. For more accurate measurements, the calculations must take the actual spheroidal shape of the world into account, and the mathematics becomes very complicated indeed.

This applies to SQL Server as well: https://docs.microsoft.com/en-us/sql/relational-databases/spatial/spatial-data-sql-server

MySQL implementation is more simplified. By default every geometry feature type is planar (SRID 0), requiring you to use appropriate functions (e.g. ST_Distance_Sphere) and SRIDs (e.g. 4326 "WGS84") to calculate over a sphere.

We are deliberately using the geography spatial type for PostgreSQL/SQL Server because it's the most accurate when calculating distances from the globe: https://gis.stackexchange.com/a/33577

So in short we can't return the geometry type and mix those two different spatial types.

The geometry method currently not supported in Laravel is about the feature type from the spatial type as defined in the OpenGIS specification. The geometry feature type is a special feature used to store any other feature type.
Indeed both spatial types have the geometry feature type, and we would want to use the geographic spatial type here: geography('geometry', 4326)

To help in the matter I just tested creating a column with geography('geometry', 4326) on PostgreSQL 10.4 and PostGIS 2.4 and it worked as expected.

With all that said, I think it would be really welcome to fix this mistake. 👍 😉

Contributor

paulofreitas commented Jul 27, 2018

Indeed, this is an interesting question. Thanks @denaje! 👍

There are two different spatial types used in PostGIS: geometry (planar) and geography (spheroid). Quoting the PostGIS documentation:

The basis for the PostGIS geometry type is a plane. The shortest path between two points on the plane is a straight line. That means calculations on geometries (areas, distances, lengths, intersections, etc) can be calculated using cartesian mathematics and straight line vectors.

The basis for the PostGIS geographic type is a sphere. The shortest path between two points on the sphere is a great circle arc. That means that calculations on geographies (areas, distances, lengths, intersections, etc) must be calculated on the sphere, using more complicated mathematics. For more accurate measurements, the calculations must take the actual spheroidal shape of the world into account, and the mathematics becomes very complicated indeed.

This applies to SQL Server as well: https://docs.microsoft.com/en-us/sql/relational-databases/spatial/spatial-data-sql-server

MySQL implementation is more simplified. By default every geometry feature type is planar (SRID 0), requiring you to use appropriate functions (e.g. ST_Distance_Sphere) and SRIDs (e.g. 4326 "WGS84") to calculate over a sphere.

We are deliberately using the geography spatial type for PostgreSQL/SQL Server because it's the most accurate when calculating distances from the globe: https://gis.stackexchange.com/a/33577

So in short we can't return the geometry type and mix those two different spatial types.

The geometry method currently not supported in Laravel is about the feature type from the spatial type as defined in the OpenGIS specification. The geometry feature type is a special feature used to store any other feature type.
Indeed both spatial types have the geometry feature type, and we would want to use the geographic spatial type here: geography('geometry', 4326)

To help in the matter I just tested creating a column with geography('geometry', 4326) on PostgreSQL 10.4 and PostGIS 2.4 and it worked as expected.

With all that said, I think it would be really welcome to fix this mistake. 👍 😉

@denaje

This comment has been minimized.

Show comment
Hide comment
@denaje

denaje Jul 27, 2018

Contributor

Thanks for the thorough analysis @paulofreitas! I was not aware about the difference between geometry and geography before. I will be happy to make a PR with the proposed change.

Contributor

denaje commented Jul 27, 2018

Thanks for the thorough analysis @paulofreitas! I was not aware about the difference between geometry and geography before. I will be happy to make a PR with the proposed change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment