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

[5.5] Geospatial improvements #21919

Merged
merged 4 commits into from
Nov 2, 2017

Conversation

paulofreitas
Copy link
Contributor

@paulofreitas paulofreitas commented Nov 2, 2017

  • Adds missing spatial columns support to SQL Server driver
  • Adds missing spatial index support to other drivers than MySQL
  • Adds missing spatial index dropping support
  • Misc improvements to geospatial support

Since it ended up being big I pushed the commits without squashing them to ease reviewing what have changed. 👍

Added 18 new tests and 39 assertions to the test suite.

Notes

I was going do add full SpatiaLite support to the SQLite driver, then I realized that unfortunately the PDO SQLite driver does not support loading SQLite extensions at all, only the PHP SQLite3 extension. 😞

Note that since SQLite data types are dynamic it does make sense to support spatial column types as already implemented. 😉

@taylorotwell
Copy link
Member

I don't like that you changed so many comments, formatting, and unrelated things. I can barely tell what is going on because of all the noise in this PR.

@paulofreitas paulofreitas force-pushed the geospatial-improvements branch from b39f45c to 283e505 Compare November 2, 2017 15:49
@paulofreitas paulofreitas force-pushed the geospatial-improvements branch from 283e505 to 416c9df Compare November 2, 2017 15:57
@paulofreitas
Copy link
Contributor Author

I've rebased discarding some code simplifications I did previously. Now only the last commit change stuff, and it's easier to review it isolated from the others.

Regarding the last commit, it just...

  • Improves grammars' docstrings and method names for spatial column types (this does not break anything, it's just to follow the convention used within other methods)
  • Improves grammars exception type/message for unsupported spatial features
  • Standardizes spatial features tests across all drivers (i. e. makes them similar)

All tests passed. Hope it's easier to review now. 😄

@taylorotwell taylorotwell merged commit 416c9df into laravel:5.5 Nov 2, 2017
@paulofreitas paulofreitas deleted the geospatial-improvements branch November 2, 2017 20:39
@antonkomarev
Copy link
Contributor

antonkomarev commented Nov 2, 2017

@paulofreitas thank you for improving Geo functionality!

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.

3 participants