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

Mysql57 geography srid #156

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

hrach
Copy link

@hrach hrach commented May 19, 2016

No description provided.

@coveralls
Copy link

coveralls commented May 19, 2016

Coverage Status

Coverage increased (+0.005%) to 99.123% when pulling c703730 on tripomatic:mysql57_geography_srid into 501df61 on creof:develop.

@hrach
Copy link
Author

hrach commented May 19, 2016

Ok, here we are, the issue is - MySQL 5.7 checks for the same SRID for the values. If they are not, it throws "General error: 3033 Binary geometry function mbrcontains given two geometries of different srids: 4326 and 0, which should have been identical.". MySQL 5.6 ignores the second parameter and inserts the geometry with srid 0.

Currently, Travis does not support MySQL 5.7, so I rather not to write sh scripts to somehow reinstall the database to properly test this. Right now, we are deploying this into production.

@hrach hrach changed the title WIP - Mysql57 geography srid Mysql57 geography srid May 19, 2016
@sadortun
Copy link
Member

sadortun commented May 19, 2016

Hi @hrach i think testing 5.7 is a good idea, not only for this PR, but for the entire project.

We should implement #157 before merging this PR

@djlambert
Copy link
Member

The issue I see with this is there's no way to change the SRID from the default. At the very least It should use the SRID if set in the object, then fallback to default.

@@ -34,6 +35,8 @@
*/
class MySql extends AbstractPlatform
{
const DEFAULT_SRID = 4326;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4326 magic number : spherical Mercator

http://spatialreference.org/ref/epsg/4326/

@hrach
Copy link
Author

hrach commented May 19, 2016

@djlambert well, I try to dig a little bit, but I wasn't able to get the while concept how the interface and methods work. In the current method I'm just chainging the raw sql, there is no object, as for Postgre. Moreover, MySQL 5.7 doesn't seems to work with different srid in one column.

We may to add some configuration for the column, but I'm not sure where to get It. One possibility is not to solve it, ale let people override the constant and use LSB.

@yurtesen
Copy link

Is there any news about if SRID's will be supported for MySQL or not?

@holtkamp
Copy link

holtkamp commented Jan 23, 2020

For PostgreSQL when writing a Geography to the database the SRID can be indicated using Extended WKT (EWKT), for example: SRID=4326;POINT(x y)

Unfortunately MySQL does not support EWKT

As @hrach suggested, the simplest approach seems to introduce a public static property which can be set by the user during ORM configuration:

\CrEOF\Spatial\DBAL\Platform\MySql::$srid = 4326;

And then read when persisting a GeographyType:

    public function convertToDatabaseValueSQL(AbstractSpatialType $type, $sqlExpr)
    {
        return $type instanceof GeographyType && is_int(self::$srid)
            ? sprintf('ST_GeomFromText(%s, %d)', $sqlExpr, self::$srid)
            : sprintf('ST_GeomFromText(%s)', $sqlExpr);
    }

Adopted this in the following branch: https://github.com/holtkamp/doctrine2-spatial/blob/mysql-80-support/lib/CrEOF/Spatial/DBAL/Platform/MySql.php, available for merge via #196

Sidenote: this rather naive approach does prevent the use of different SRIDs in the same database...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants