From c6b698cb027a14e9b0a2ce3e403ce12d1bc132d3 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 7 Apr 2020 08:26:11 +1200 Subject: [PATCH 1/3] NEW Allow InnoDB for FULLTEXT indexes MyISAM used to be the only one to support it, now InnoDB has caught up. Unless an engine is set specifically in create_table_options, this will auto-convert existing MyISAM tables to InnoDb. Fixes #9242 --- docs/en/04_Changelogs/4.6.0.md | 55 +++++++++++++++++++ src/ORM/Connect/MySQLSchemaManager.php | 33 ++--------- src/ORM/Filters/FulltextFilter.php | 4 +- .../ORM/DataObjectSchemaGenerationTest.php | 6 -- .../Filters/FulltextFilterTest/TestObject.php | 3 - 5 files changed, 61 insertions(+), 40 deletions(-) create mode 100644 docs/en/04_Changelogs/4.6.0.md diff --git a/docs/en/04_Changelogs/4.6.0.md b/docs/en/04_Changelogs/4.6.0.md new file mode 100644 index 000000000..b2ca79cba --- /dev/null +++ b/docs/en/04_Changelogs/4.6.0.md @@ -0,0 +1,55 @@ +# 4.6.0 (Unreleased) + +## Overview {#overview} + + * [MySQL tables are auto-converted from MyISAM to InnoDB](#myisam) + +## MySQL tables are auto-converted from MyISAM to InnoDB {#myisam} + +Beginning with [4.4.0](https://docs.silverstripe.org/en/4/changelogs/4.4.0/), +our minimum requirement for MySQL is 5.6 (since MySQL 5.5 end of life reached in December 2018). +Starting with MySQL 5.6, [InnoDB](https://dev.mysql.com/doc/refman/5.6/en/innodb-introduction.html) +is the new default storage engine, +replacing the older [MyISAM](https://dev.mysql.com/doc/refman/5.6/en/myisam-storage-engine.html) engine. + +Silverstripe CMS already creates InnoDB tables by default, +mainly in order to benefit from their better support for database transactions. +Before MySQL 5.6, InnoDB didn't have a `FULLTEXT` search index, +requiring us to enforce the MyISAM engine when devs opted into this index type +in their particular setup. There are a few ways in which this opt-in can happen: + + * Adding the [FulltextSearchable](https://github.com/silverstripe/silverstripe-framework/blob/4/src/ORM/Search/FulltextSearchable.php) + extension to a DataObject, as described in our + [search docs](https://docs.silverstripe.org/en/4/developer_guides/search/fulltextsearch/) + * Defining `'type' => 'fulltext'` in `DataObject::$db` column definitions + * Implementing [DBIndexable](https://github.com/silverstripe/silverstripe-framework/blob/4/src/ORM/FieldType/DBIndexable.php) + on a custom `DBField` subclass. + * Setting `'ENGINE=MyISAM'` in `DataObject::$create_table_options` + +This search index is not required to enable simple text search +in the "Pages" section of the CMS, or any ModelAdmin implementations. +We generally recommend to choose a more powerful +[search addon](https://addons.silverstripe.org/add-ons?search=fulltext&type=&sort=downloads) +(e.g. based on Solr or ElasticSearch) for website frontend search use cases. + +As of 4.6.0, a `dev/build` will automatically switch MyISAM tables to InnoDB, +which automatically recreates any indexes required. If you have large indexes, +this can extend the duration if this task. As usual, back up your database +before upgrading, and test upgrades on non-production systems first. +Our tests indicate that indexes with thousands of records and screen pages +worth of content (15MB index size) are converted in a few seconds. + +In order to opt out of this change, you can set the engine explicitly +for your DataObject implementations: + +```php +use SilverStripe\ORM\Connect\MySQLSchemaManager; +use SilverStripe\ORM\DataObject; + +class MyDataObject extends DataObject +{ + private static $create_table_options = [ + MySQLSchemaManager::ID => 'ENGINE=MyISAM' + ]; +} +``` \ No newline at end of file diff --git a/src/ORM/Connect/MySQLSchemaManager.php b/src/ORM/Connect/MySQLSchemaManager.php index 65b53dbf1..10334e302 100644 --- a/src/ORM/Connect/MySQLSchemaManager.php +++ b/src/ORM/Connect/MySQLSchemaManager.php @@ -40,10 +40,6 @@ class MySQLSchemaManager extends DBSchemaManager } if ($indexes) { foreach ($indexes as $k => $v) { - // force MyISAM if we have a fulltext index - if ($v['type'] === 'fulltext') { - $addOptions = 'ENGINE=MyISAM'; - } $indexSchemas .= $this->getIndexSqlDefinition($k, $v) . ",\n"; } } @@ -104,30 +100,11 @@ class MySQLSchemaManager extends DBSchemaManager $dbID = self::ID; if ($alteredOptions && isset($alteredOptions[$dbID])) { - $indexList = $this->indexList($tableName); - $skip = false; - foreach ($indexList as $index) { - if ($index['type'] === 'fulltext') { - $skip = true; - break; - } - } - if ($skip) { - $this->alterationMessage( - sprintf( - "Table %s options not changed to %s due to fulltextsearch index", - $tableName, - $alteredOptions[$dbID] - ), - "changed" - ); - } else { - $this->query(sprintf("ALTER TABLE \"%s\" %s", $tableName, $alteredOptions[$dbID])); - $this->alterationMessage( - sprintf("Table %s options changed: %s", $tableName, $alteredOptions[$dbID]), - "changed" - ); - } + $this->query(sprintf("ALTER TABLE \"%s\" %s", $tableName, $alteredOptions[$dbID])); + $this->alterationMessage( + sprintf("Table %s options changed: %s", $tableName, $alteredOptions[$dbID]), + "changed" + ); } $alterations = implode(",\n", $alterList); diff --git a/src/ORM/Filters/FulltextFilter.php b/src/ORM/Filters/FulltextFilter.php index ff47362ff..2248646e6 100755 --- a/src/ORM/Filters/FulltextFilter.php +++ b/src/ORM/Filters/FulltextFilter.php @@ -2,7 +2,6 @@ namespace SilverStripe\ORM\Filters; -use SilverStripe\Core\Convert; use SilverStripe\ORM\DataQuery; use SilverStripe\ORM\DataObject; use Exception; @@ -10,8 +9,7 @@ use Exception; /** * Filters by full-text matching on the given field. * - * Full-text indexes are only available with MyISAM tables. The following column types are - * supported: + * The following column types are supported: * - Char * - Varchar * - Text diff --git a/tests/php/ORM/DataObjectSchemaGenerationTest.php b/tests/php/ORM/DataObjectSchemaGenerationTest.php index 73313d63b..eeabc2c46 100644 --- a/tests/php/ORM/DataObjectSchemaGenerationTest.php +++ b/tests/php/ORM/DataObjectSchemaGenerationTest.php @@ -25,12 +25,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest // Start tests static::start(); - // enable fulltext option on this table - TestIndexObject::config()->update( - 'create_table_options', - array(MySQLSchemaManager::ID => 'ENGINE=MyISAM') - ); - parent::setUpBeforeClass(); } diff --git a/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php b/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php index adce29c48..4ba2cc4f9 100644 --- a/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php +++ b/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php @@ -35,7 +35,4 @@ class TestObject extends DataObject implements TestOnly ], ); - private static $create_table_options = array( - MySQLSchemaManager::ID => "ENGINE=MyISAM", - ); } From 0215fdd26254d36ee3ce491a5afb47d6fa97f5a3 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 8 Apr 2020 14:52:21 +1200 Subject: [PATCH 2/3] DOC Clarify sanitisation in searchEngine() under boolean mode This came up in https://github.com/silverstripe/silverstripe-cms/issues/1452, and wasn't fully addressed. Either we allow boolean mode and all the constraints this brings around special character usage, or we filter out those special characters, which makes boolean mode pointless. You can't just pass arbitrary user input in a power-user function like this. See https://dev.mysql.com/doc/refman/5.6/en/fulltext-boolean.html Context: This used to work for some examples like "foo>*" under MyISAM, presumably because it had a more lenient parser. InnoDB rightfully complains about this now. --- src/ORM/Connect/MySQLDatabase.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php index c19004885..7469e4292 100644 --- a/src/ORM/Connect/MySQLDatabase.php +++ b/src/ORM/Connect/MySQLDatabase.php @@ -144,6 +144,12 @@ class MySQLDatabase extends Database implements TransactionManager * The core search engine, used by this class and its subclasses to do fun stuff. * Searches both SiteTree and File. * + * Caution: While the $keywords argument is escaped for safe use in a query context, + * you need to ensure that it is also a valid boolean expression when opting into $booleanSearch. + * For example, the "asterisk" and "greater than" characters have a special meaning in this context, + * and can only be placed in certain parts of the keywords. You will need to preprocess and sanitise + * user input accordingly in order to avoid query errors. + * * @param array $classesToSearch * @param string $keywords Keywords as a string. * @param int $start From 2c5deceeb475a6842c30dd42ff3a9990dda50707 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 9 Apr 2020 10:30:37 +1200 Subject: [PATCH 3/3] FIX Filter out all FULLTEXT BOOLEAN chars The query might still work depending on where these chars are placed, but it seems weird to only remove *some* of the valid chars here. See https://dev.mysql.com/doc/refman/5.6/en/fulltext-boolean.html Note that the query runs both the actual boolean query with chars, and then a separate relevance search without them. --- src/ORM/Connect/MySQLDatabase.php | 5 +++-- tests/php/ORM/Filters/FulltextFilterTest/TestObject.php | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php index 7469e4292..e5c50d1c4 100644 --- a/src/ORM/Connect/MySQLDatabase.php +++ b/src/ORM/Connect/MySQLDatabase.php @@ -227,8 +227,9 @@ class MySQLDatabase extends Database implements TransactionManager $match[$fileClass] = "MATCH (Name, Title) AGAINST ('$keywords' $boolean) AND ClassName = '$fileClassSQL'"; // We make the relevance search by converting a boolean mode search into a normal one - $relevanceKeywords = str_replace(array('*', '+', '-'), '', $keywords); - $htmlEntityRelevanceKeywords = str_replace(array('*', '+', '-'), '', $htmlEntityKeywords); + $booleanChars = ['*', '+', '@', '-', '(', ')', '<', '>']; + $relevanceKeywords = str_replace($booleanChars, '', $keywords); + $htmlEntityRelevanceKeywords = str_replace($booleanChars, '', $htmlEntityKeywords); $relevance[$pageClass] = "MATCH (Title, MenuTitle, Content, MetaDescription) " . "AGAINST ('$relevanceKeywords') " . "+ MATCH (Title, MenuTitle, Content, MetaDescription) AGAINST ('$htmlEntityRelevanceKeywords')"; diff --git a/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php b/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php index 4ba2cc4f9..68b74e501 100644 --- a/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php +++ b/tests/php/ORM/Filters/FulltextFilterTest/TestObject.php @@ -34,5 +34,4 @@ class TestObject extends DataObject implements TestOnly 'columns' => ['ColumnE'], ], ); - }