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

Add options.autoIncrement to MySQL #3076

Merged
merged 7 commits into from Feb 9, 2015

Conversation

3 participants
@mdpauley
Contributor

mdpauley commented Feb 9, 2015

The initial AUTO_INCREMENT value for the table.

Add options.autoIncrement to MySQL
The initial AUTO_INCREMENT value for the table.
@janmeier

This comment has been minimized.

Member

janmeier commented Feb 9, 2015

Needs a test, and the option should be added to the API docs for sequelize.define

@mdpauley

This comment has been minimized.

Contributor

mdpauley commented Feb 9, 2015

Sorry about that! Added the test and to the API docs.

},
{
arguments: ['myTable', {id: 'INTEGER auto_increment PRIMARY KEY'}, {initialAutoIncrement: 1000001}],
expectation: 'CREATE TABLE IF NOT EXISTS `myTable` (`id` INTEGER auto_increment , PRIMARY KEY (`id`)) ENGINE=InnoDB AUTO_INCREMENT=1000001;'

This comment has been minimized.

@mickhansen

mickhansen Feb 9, 2015

Contributor

Why the multiple spaces?

This comment has been minimized.

@mdpauley

mdpauley Feb 9, 2015

Contributor

They are due to the spaces in query variable between charset and collation that weren't set in that example. In my opinion, a proper fix would be to add the spaces in the values object, like with comment, when charset and collation are generated.

Example:

var query = 'CREATE TABLE IF NOT EXISTS <%= table %> (<%= attributes%>)<%= comment %> ENGINE=<%= engine %><%= charset %><%= collation %><%= initialAutoIncrement %>'
   , primaryKeys = []
   , foreignKeys = {}
   , attrStr = [];

and

var values = {
  table: this.quoteTable(tableName),
  attributes: attrStr.join(', '),
  comment: options.comment && Utils._.isString(options.comment) ? ' COMMENT ' + this.escape(options.comment) : '',
  engine: options.engine,
  charset: (options.charset ? ' DEFAULT CHARSET=' + options.charset : ''),
  collation: (options.collate ? ' COLLATE ' + options.collate : ''),
  initialAutoIncrement: (options.initialAutoIncrement ? ' AUTO_INCREMENT=' + options.initialAutoIncrement : '')
}

I could make those changes and include with this pull request.

This comment has been minimized.

@mickhansen

mickhansen Feb 9, 2015

Contributor

@mdpauley if you are up for it that would be welcome. But not critical for the merge of this PR.

@@ -482,6 +482,7 @@ For more about validation, see http://sequelizejs.com/docs/latest/models#validat
| [options.charset] | String | |
| [options.comment] | String | |
| [options.collate] | String | |
| [options.initialAutoIncrement] | String | Set the initial AUTO_INCREMENT value for the table in MySQL. |

This comment has been minimized.

@mickhansen

mickhansen Feb 9, 2015

Contributor

You have to add the docs to the code, docs/api is auto generated

mdpauley and others added some commits Feb 9, 2015

Remove spaces from createTableQuery query variable
and added spaces to beginning of charset, collation & initialAutoIncrement in values during string generation
mdpauley
@mdpauley

This comment has been minimized.

Contributor

mdpauley commented Feb 9, 2015

@janmeier & @mickhansen everything should be up to standard now.

janmeier added a commit that referenced this pull request Feb 9, 2015

Merge pull request #3076 from mdpauley/master
Add options.autoIncrement to MySQL

@janmeier janmeier merged commit a9c500c into sequelize:master Feb 9, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

janmeier added a commit that referenced this pull request Feb 9, 2015

@janmeier

This comment has been minimized.

Member

janmeier commented Feb 9, 2015

Sweet, thanks @mdpauley :)

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