From 93eab9c132d002cb425909dbb030a5aa5cfb9a7e Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 24 Sep 2012 15:36:59 +1200 Subject: [PATCH] FIXED: Repaired failing test cases (DataObjectSchemaGenerationTest) FIXED: Incorrect implementation of alterIndex UPDATED: Improved parsing of index specification by including code borrowed from the postgres database connector module. Possibly a candidate for refactor into SS_Database. --- code/MSSQLDatabase.php | 220 ++++++++++++++++++++++++++++------------- 1 file changed, 153 insertions(+), 67 deletions(-) diff --git a/code/MSSQLDatabase.php b/code/MSSQLDatabase.php index 104d742..1f0e427 100644 --- a/code/MSSQLDatabase.php +++ b/code/MSSQLDatabase.php @@ -587,8 +587,7 @@ class MSSQLDatabase extends SS_Database { $bits = preg_split('/ *= */', $segment); for($i = 1; $i < sizeof($bits); $i += 2) { array_unshift($constraints, substr(rtrim($bits[$i], ')'), 1, -1)); - - } + } } return $constraints; } @@ -688,8 +687,8 @@ class MSSQLDatabase extends SS_Database { * @param string $newName The new name of the field */ public function renameField($tableName, $oldName, $newName) { - $this->query("EXEC sp_rename @objname = '$tableName.$oldName', @newname = '$newName', @objtype = 'COLUMN'"); - } + $this->query("EXEC sp_rename @objname = '$tableName.$oldName', @newname = '$newName', @objtype = 'COLUMN'"); + } public function fieldList($table) { //This gets us more information than we need, but I've included it all for the moment.... @@ -815,56 +814,146 @@ class MSSQLDatabase extends SS_Database { * arrays to be created. */ public function convertIndexSpec($indexSpec){ - if(is_array($indexSpec)){ - //Here we create a db-specific version of whatever index we need to create. - switch($indexSpec['type']){ - case 'fulltext': - $indexSpec='fulltext (' . str_replace(' ', '', $indexSpec['value']) . ')'; - break; - case 'unique': - $indexSpec='unique (' . $indexSpec['value'] . ')'; - break; - } + $indexSpec = $this->parseIndexSpec(null, $indexSpec); + return "{$indexSpec['type']} ({$indexSpec['value']})"; + } + + /** + * Splits a spec string safely, considering quoted columns, whitespace, + * and cleaning brackets + * @param string $spec The input index specification + * @return array List of columns in the spec + */ + function explodeColumnString($spec) { + // Remove any leading/trailing brackets and outlying modifiers + // E.g. 'unique (Title, "QuotedColumn");' => 'Title, "QuotedColumn"' + $containedSpec = preg_replace('/(.*\(\s*)|(\s*\).*)/', '', $spec); + + // Split potentially quoted modifiers + // E.g. 'Title, "QuotedColumn"' => array('Title', 'QuotedColumn') + return preg_split('/"?\s*,\s*"?/', trim($containedSpec, '(") ')); + } + + /** + * Builds a properly quoted column list from an array + * @param array $columns List of columns to implode + * @return string A properly quoted list of column names + */ + function implodeColumnList($columns) { + if(empty($columns)) return ''; + return '"' . implode('","', $columns) . '"'; + } + + /** + * Given an index specification in the form of a string ensure that each + * column name is property quoted, stripping brackets and modifiers. + * This index may also be in the form of a "CREATE INDEX..." sql fragment + * @param string $spec The input specification or query. E.g. 'unique (Column1, Column2)' + * @return string The properly quoted column list. E.g. '"Column1", "Column2"' + */ + function quoteColumnSpecString($spec) { + $bits = $this->explodeColumnString($spec); + return $this->implodeColumnList($bits); + } + + /** + * Given an index spec determines the index type + * @param type $spec + * @return string + */ + function determineIndexType($spec) { + // check array spec + if(is_array($spec) && isset($spec['type'])) { + return $spec['type']; + } elseif (!is_array($spec) && preg_match('/(?\w+)\s*\(/', $spec, $matchType)) { + return strtolower($matchType['type']); + } else { + return 'index'; } + } + + /** + * Converts an array or string index spec into a universally useful array + * @see convertIndexSpec() for approximate inverse + * @param string|array $spec An index specification in either one of two formats: + * + * Index types may be one of unique, index, fulltext, or any adaptor specific + * format. Column names may be double quoted. + * @return array The resulting spec array with the required fields 'name', + * 'type', and 'value'. The index value will have cleanly quoted column names. + * E.g. array( + * 'type' => 'unique', + * 'name' => 'MyIndex', + * 'value' => '"Title", "Content"' + * ) + */ + function parseIndexSpec($name, $spec) { + + // Do minimal cleanup on any already parsed spec + if(is_array($spec)) { + $spec['value'] = $this->quoteColumnSpecString($spec['value']); + return $spec; + } + + // Nicely formatted spec! + return array( + 'name' => $name, + 'value' => $this->quoteColumnSpecString($spec), + 'type' => $this->determineIndexType($spec) + ); + } - return $indexSpec; + /** + * Builds the internal MS SQL Server index name given the silverstripe table and index name + * @param string $tableName + * @param string $indexName + * @param string $prefix The optional prefix for the index. Defaults to "ix" for indexes. + * @return string The postgres name of the index + */ + function buildMSSQLIndexName($tableName, $indexName, $prefix = 'ix') { + + // Cleanup names of namespaced tables + $tableName = str_replace('\\', '_', $tableName); + + return "{$prefix}_{$tableName}_{$indexName}"; } /** * Return SQL for dropping and recreating an index */ protected function getIndexSqlDefinition($tableName, $indexName, $indexSpec) { - $index = 'ix_' . str_replace('\\', '_', $tableName) . '_' . $indexName; + + // Determine index name + $index = $this->buildMSSQLIndexName($tableName, $indexName); + + // Consolidate/Cleanup spec into array format + $indexSpec = $this->parseIndexSpec($indexName, $indexSpec); + $drop = "IF EXISTS (SELECT name FROM sys.indexes WHERE name = '$index') DROP INDEX $index ON \"" . $tableName . "\";"; - if(!is_array($indexSpec)) { - $indexSpec=trim($indexSpec, '()'); - return "$drop CREATE INDEX $index ON \"" . $tableName . "\" (" . $indexSpec . ");"; - } else { - // create a type-specific index - if($indexSpec['type'] == 'fulltext') { - if($this->fullTextEnabled()) { - // enable fulltext on this table - $this->createFullTextCatalog(); - $primary_key = $this->getPrimaryKey($tableName); + // create a type-specific index + if($indexSpec['type'] == 'fulltext' && $this->fullTextEnabled()) { + // enable fulltext on this table + $this->createFullTextCatalog(); + $primary_key = $this->getPrimaryKey($tableName); - $query = ''; - if($primary_key) { - $query .= "CREATE FULLTEXT INDEX ON \"$tableName\" ({$indexSpec['value']}) KEY INDEX $primary_key WITH CHANGE_TRACKING AUTO;"; - } - - return $query; - } - } - - if($indexSpec['type'] == 'unique') { - if(!is_array($indexSpec['value'])) $columns = preg_split('/ *, */', trim($indexSpec['value'])); - else $columns = $indexSpec['value']; - $SQL_columnList = implode(', ', $columns); - - return "$drop CREATE UNIQUE INDEX $index ON \"" . $tableName . "\" ($SQL_columnList);"; + if($primary_key) { + return "$drop CREATE FULLTEXT INDEX ON \"$tableName\" ({$indexSpec['value']})" + . "KEY INDEX $primary_key WITH CHANGE_TRACKING AUTO;"; } } + + if($indexSpec['type'] == 'unique') { + return "$drop CREATE UNIQUE INDEX $index ON \"$tableName\" ({$indexSpec['value']});"; + } + + return "$drop CREATE INDEX $index ON \"$tableName\" ({$indexSpec['value']});"; } function getDbSqlDefinition($tableName, $indexName, $indexSpec){ @@ -878,19 +967,7 @@ class MSSQLDatabase extends SS_Database { * @param string $indexSpec The specification of the index, see SS_Database::requireIndex() for more details. */ public function alterIndex($tableName, $indexName, $indexSpec) { - $indexSpec = trim($indexSpec); - if($indexSpec[0] != '(') { - list($indexType, $indexFields) = explode(' ',$indexSpec,2); - } else { - $indexFields = $indexSpec; - } - - if(!$indexType) { - $indexType = "index"; - } - - $this->query("DROP INDEX $indexName ON $tableName;"); - $this->query("ALTER TABLE \"$tableName\" ADD $indexType \"$indexName\" $indexFields"); + $this->createIndex($tableName, $indexName, $indexSpec); } /** @@ -900,33 +977,43 @@ class MSSQLDatabase extends SS_Database { */ public function indexList($table) { $indexes = DB::query("EXEC sp_helpindex '$table';"); - $prefix = ''; $indexList = array(); - + + // Enumerate all basic indexes foreach($indexes as $index) { if(strpos($index['index_description'], 'unique') !== false) { - $prefix='unique '; + $prefix = 'unique '; + } else { + $prefix= 'index '; } + + // Extract name from index + $baseIndexName = $this->buildMSSQLIndexName($table, ''); + $indexName = substr($index['index_name'], strlen($baseIndexName)); + + // Extract columns + $columns = $this->quoteColumnSpecString($index['index_keys']); - $key = str_replace(', ', ',', $index['index_keys']); - $indexList[$key]['indexname'] = $index['index_name']; - $indexList[$key]['spec'] = $prefix . '(' . $key . ')'; + $indexList[$indexName]['indexname'] = $indexName; + $indexList[$indexName]['spec'] = "$prefix($columns)"; } // Now we need to check to see if we have any fulltext indexes attached to this table: if($this->fullTextEnabled()) { $result = DB::query('EXEC sp_help_fulltext_columns;'); - $columns = ''; + + // Extract columns from this fulltext definition + $columns = array(); foreach($result as $row) { if($row['TABLE_NAME'] == $table) { - $columns .= $row['FULLTEXT_COLUMN_NAME'] . ','; + $columns[] = $row['FULLTEXT_COLUMN_NAME']; } } - if($columns!=''){ - $columns=trim($columns, ','); + if(!empty($columns)) { + $columns = $this->implodeColumnList($columns); $indexList['SearchFields']['indexname'] = 'SearchFields'; - $indexList['SearchFields']['spec'] = 'fulltext (' . $columns . ')'; + $indexList['SearchFields']['spec'] = "fulltext ($columns)"; } } @@ -1282,10 +1369,9 @@ class MSSQLDatabase extends SS_Database { /** * This changes the index name depending on database requirements. - * MSSQL requires underscores to be replaced with commas. */ function modifyIndex($index) { - return str_replace('_', ',', $index); + return $index; } /**