Allow mysql spatial indexes #541

Merged
merged 2 commits into from Mar 25, 2014

Conversation

Projects
None yet
5 participants
Contributor

jsor commented Mar 6, 2014

Allows spatial indexes via the SPATIAL flag.
http://dev.mysql.com/doc/refman/5.7/en/creating-spatial-indexes.html

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-832

We use Jira to track the state of pull requests and the versions they got
included in.

Member

deeky666 commented Mar 6, 2014

The schema manager part for reverse engineering the SPATIAL flag on indexes is missing. You should implement it here. And please also add a functional test for this.

@@ -718,6 +718,8 @@ protected function getCreateIndexSQLFlags(Index $index)
$type .= 'UNIQUE ';
} elseif ($index->hasFlag('fulltext')) {
$type .= 'FULLTEXT ';
+ } elseif ($index->hasFlag('spatial')) {
+ $type .= 'SPATIAL ';
@deeky666

deeky666 Mar 6, 2014

Member

Just a side note. I'm not quite happy with this overall implementation in this method as those flags are engine specific and should only be evaluated if the corresponding table uses the MyISAM engine. Also only one of those flags is allowed for an index and IMO an exception should be thrown here if for example both FULLTEXT and SPATIAL flags are given rather than using the first match.
This is of course not part of this PR and not your fault. Just wanted to share my opinion on the overall approach :)

@intellix

intellix Jun 14, 2014

Something I've just stumbled upon as I'm looking for SPATIAL indexes in Doctrine: http://mysqlserverteam.com/innodb-spatial-indexes-in-5-7-4-lab-release/
5.7.4 LAB release says that it supports Spatial indexes

Contributor

jsor commented Mar 6, 2014

Thanks for the feedback. Reverse engineering part added.

+ return strtoupper($this->getName());
+ }
+
+ public function getMappedDatabaseTypes(AbstractPlatform $platform)
@deeky666

deeky666 Mar 6, 2014

Member

What's this method good for? It is not used anywhere in the tests.

@jsor

jsor Mar 6, 2014

Contributor

It's used by the SchemaManager while deleting the test tables.

@jsor

jsor Mar 6, 2014

Contributor

Without it, these two tests where failing with Exception: [Doctrine\DBAL\DBALException] Unknown database type point requested, Doctrine\DBAL\Platforms\MySqlPlatform may not support it.

https://github.com/doctrine/dbal/blob/master/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php#L97
https://github.com/doctrine/dbal/blob/master/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php#L368

@deeky666

deeky666 Mar 6, 2014

Member

Damn, you're right. Wasn't aware of that method. Thanks for pointing out :)

@beberlei This patch is okay from my side. 👍

beberlei added a commit that referenced this pull request Mar 25, 2014

@beberlei beberlei merged commit b693433 into doctrine:master Mar 25, 2014

1 check passed

default The Travis CI build passed
Details

@jsor jsor deleted the jsor:mysql-spatial-index branch Mar 25, 2014

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