From ac8f882175e9d9625c73d18104eb02ffef0302a8 Mon Sep 17 00:00:00 2001 From: dgsan Date: Thu, 14 Feb 2013 09:37:32 -0700 Subject: [PATCH] This adds support for indexes to automigrate() and adds a check for the indexes existence to the migration.coffee 'should run migration' test. All unit tests continue to pass. The code snippets for adding indexes are the declarative versions of the methods used in alterTable(), so the indexes should be consistant with previous declarations/additions. Should resolve bug #5. As a side node, also removed the 'UNIQUE' clause from the `id` declaration as it is redundant with the 'PRIMARY KEY' declaration so far as I'm aware and results in what seems to be a second index getting maintained for no reason. (If I'm wrong about this I'd be intrested in the explanation. MySQL does have a lot of oddities.) --- lib/mysql.js | 51 ++++++++++++++++++++++++++++++++++++++++++- test/migration.coffee | 50 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/lib/mysql.js b/lib/mysql.js index f8a7caa..1399193 100644 --- a/lib/mysql.js +++ b/lib/mysql.js @@ -510,13 +510,62 @@ MySQL.prototype.alterTable = function (model, actualFields, actualIndexes, done, MySQL.prototype.propertiesSQL = function (model) { var self = this; - var sql = ['`id` INT(11) NOT NULL AUTO_INCREMENT UNIQUE PRIMARY KEY']; + var sql = ['`id` INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY']; Object.keys(this._models[model].properties).forEach(function (prop) { if (prop === 'id') return; sql.push('`' + prop + '` ' + self.propertySettingsSQL(model, prop)); }); + // Declared in model index property indexes. + Object.keys(this._models[model].properties).forEach(function (prop) { + var i = self._models[model].properties[prop].index; + if (i) { + sql.push(self.singleIndexSettingsSQL(model, prop)); + } + }); + // Settings might not have an indexes property. + var dxs = this._models[model].settings.indexes; + if(dxs){ + Object.keys(this._models[model].settings.indexes).forEach(function(prop){ + sql.push(self.indexSettingsSQL(model, prop)); + }); + } return sql.join(',\n '); +}; +MySQL.prototype.singleIndexSettingsSQL = function (model, prop) { + // Recycled from alterTable single indexes above, more or less. + var i = this._models[model].properties[prop].index; + var type = ''; + var kind = ''; + if (i.type) { + type = 'USING ' + i.type; + } + if (i.kind) { + // kind = i.kind; + } + if (kind && type) { + return (kind + ' INDEX `' + prop + '` (`' + prop + '`) ' + type); + } else { + return (kind + ' INDEX `' + prop + '` ' + type + ' (`' + prop + '`) '); + } +} + +MySQL.prototype.indexSettingsSQL = function (model, prop) { + // Recycled from alterTable multi-column indexes above, more or less. + var i = this._models[model].settings.indexes[prop]; + var type = ''; + var kind = ''; + if (i.type) { + type = 'USING ' + i.kind; + } + if (i.kind) { + kind = i.kind; + } + if (kind && type) { + return (kind + ' INDEX `' + prop + '` (' + i.columns + ') ' + type); + } else { + return (kind + ' INDEX ' + type + ' `' + prop + '` (' + i.columns + ')'); + } }; MySQL.prototype.propertySettingsSQL = function (model, prop) { diff --git a/test/migration.coffee b/test/migration.coffee index f9da62a..dad8ce8 100644 --- a/test/migration.coffee +++ b/test/migration.coffee @@ -42,12 +42,12 @@ getFields = (model, cb) -> getIndexes = (model, cb) -> query 'SHOW INDEXES FROM ' + model, (err, res) -> - console.log(arguments) if err console.log err cb err else indexes = {} + # Note: this will only show the first key of compound keys res.forEach (index) -> indexes[index.Key_name] = index if parseInt(index.Seq_in_index, 10) == 1 cb err, indexes @@ -71,7 +71,7 @@ it 'should run migration', (test) -> Field: 'email' Type: 'varchar(255)' Null: 'NO' - Key: '' + Key: 'MUL' Default: null Extra: '' name: @@ -116,7 +116,51 @@ it 'should run migration', (test) -> Key: '' Default: null Extra: '' - + # Once gain, getIdexes truncates multi-key indexes to the first member. Hence index1 is correct. + getIndexes 'User', (err, fields) -> + test.deepEqual fields, + PRIMARY: + Table: 'User' + Non_unique: 0 + Key_name: 'PRIMARY' + Seq_in_index: 1 + Column_name: 'id' + Collation: 'A' + Cardinality: 0 + Sub_part: null + Packed: null + Null: '' + Index_type: 'BTREE' + Comment: '' + Index_comment: '' + email: + Table: 'User' + Non_unique: 1 + Key_name: 'email' + Seq_in_index: 1 + Column_name: 'email' + Collation: 'A' + Cardinality: 0 + Sub_part: null + Packed: null + Null: '' + Index_type: 'BTREE' + Comment: '' + Index_comment: '' + index1: + Table: 'User' + Non_unique: 1 + Key_name: 'index1' + Seq_in_index: 1 + Column_name: 'email' + Collation: 'A' + Cardinality: 0 + Sub_part: null + Packed: null + Null: '' + Index_type: 'BTREE' + Comment: '' + Index_comment: '' test.done() it 'should autoupgrade', (test) ->