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

Adds support for SRID. #111

Merged
merged 24 commits into from
Feb 29, 2020
Merged

Adds support for SRID. #111

merged 24 commits into from
Feb 29, 2020

Conversation

sikhlana
Copy link
Contributor

@sikhlana sikhlana commented Dec 7, 2019

This PR adds full support of MySQL's SRID by implementing a simple EWKB parser on top of what is currently being used. More information can be found here: #54 (comment)

@sikhlana
Copy link
Contributor Author

sikhlana commented Dec 8, 2019

Do note that this PR bumps the MySQL version to 8.0 since older versions do not have or have minimal support for SRID.

@sikhlana
Copy link
Contributor Author

sikhlana commented Dec 8, 2019

And the minimum Laravel version if 5.6 since SRID support for schema blueprint is available from then on.

…hat the first axis is longitude and the second is latitude. Adds SRID to all the eloquent scopes.
@denkurbatov
Copy link

Is it any news for when this PR will be merged to master?

Thanks in advance.

@sikhlana
Copy link
Contributor Author

@denkurbatov do note that there are a few caveats with this PR including a database-level one on how the latitude and longitude is saved as EWKB. I have been using this version on my production server and had to rebuild the spatial fields as SRID 4326 requires you to set the first value as latitude and the second as longitude in MySQL 8.0.

@grimzy grimzy changed the base branch from master to srid February 29, 2020 18:44
@grimzy
Copy link
Owner

grimzy commented Feb 29, 2020

Thank you @sikhlana!

Sorry for taking such a long time to look at this.

This article explains a lot about this PR: https://mysqlserverteam.com/axis-order-in-spatial-reference-systems/

I also understand we have to start a mysql-5.7 branch because unfortunately there's no backward compatibility. (master will be for MySQL 8)
So release v3.0.0 will be for MySQL 8 and SRID support; the 2.* versions will be for MySQL 5.7 only.

With MySQL 8's WKT now defaulting to srid-defined, it feels weird inserting long-lat + axis-order=long-lat instead of just inserting lat-long or let the users decide. (I'll ticket this)

Regarding the migrations VS Laravel 5.6, I believe this commit and maybe one more change to Grimzy\LaravelMysqlSpatial\Schema\Grammars\MySqlGrammar (+ fix tests) add what's needed for Laravel 5.2+ MySQL 8.
I'd rather not drop compatibility for older versions of Laravel and PHP until Laravel completely does (Aug 30th, 2020).

I'll be merging this with the srid branch and work on releasing v3.

Thanks again @sikhlana!

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.

4 participants