Merge pull request #2651 from sequelize/hotfix/primary-key-allow-null

Fix allowNull in combination with primaryKey

lib/dialects/sqlite/query-generator.js

@@ -38,8 +38,8 @@ module.exports = (function() {
       var query = 'CREATE TABLE IF NOT EXISTS <%= table %> (<%= attributes%>)'
       var query = 'CREATE TABLE IF NOT EXISTS <%= table %> (<%= attributes%>)'
         , primaryKeys = []
         , primaryKeys = []
         , needsMultiplePrimaryKeys = (Utils._.values(attributes).filter(function(definition) {
         , needsMultiplePrimaryKeys = (Utils._.values(attributes).filter(function(definition) {
-                    return Utils._.includes(definition, 'PRIMARY KEY');
-                  }).length > 1)
+            return Utils._.includes(definition, 'PRIMARY KEY');
+          }).length > 1)
         , attrStr = []
         , attrStr = []
         , modifierLastIndex = -1;
         , modifierLastIndex = -1;
 
 
@@ -276,7 +276,7 @@ module.exports = (function() {
             }
             }
           }
           }
 
 
-          if (dataType.hasOwnProperty('allowNull') && !dataType.allowNull && !dataType.primaryKey) {
+          if (dataType.hasOwnProperty('allowNull') && !dataType.allowNull) {
             template += " NOT NULL";
             template += " NOT NULL";
           }
           }
 
 

test/model/attributes.test.js

@@ -134,6 +134,31 @@ describe(Support.getTestDialectTeaser("Model"), function () {
         ]);
         ]);
       });
       });
 
 
+      describe('primaryKey', function () {
+        describe('in combination with allowNull', function () {
+          beforeEach(function () {
+            this.ModelUnderTest = this.sequelize.define('ModelUnderTest', {
+              identifier: {
+                primaryKey: true,
+                type:       Sequelize.STRING,
+                allowNull: false
+              }
+            });
+
+            return this.ModelUnderTest.sync({ force: true });
+          });
+
+          it('sets the column to not allow null', function () {
+            return this
+              .ModelUnderTest
+              .describe()
+              .then(function (fields) {
+                expect(fields.identifier).to.include({ allowNull: false });
+              });
+          });
+        });
+      });
+
       describe('field and attribute name is the same', function () {
       describe('field and attribute name is the same', function () {
         beforeEach(function () {
         beforeEach(function () {
           return this.Comment.bulkCreate([
           return this.Comment.bulkCreate([