bug(scope/include) Don't add an aliasses primary key to include.attributes if it…

…'s already there. Closes #4127

changelog.md

@@ -4,6 +4,7 @@
 - [FIXED] Fix save to be noop when nothing changed
 - [FIXED] Fix save to be noop when nothing changed
 - [FIXED] Call `conformOptions` on default scope [#4157](https://github.com/sequelize/sequelize/issues/4157)
 - [FIXED] Call `conformOptions` on default scope [#4157](https://github.com/sequelize/sequelize/issues/4157)
 - [FIXED] Call `conformOptions` on scopes returned by functions [#3991](https://github.com/sequelize/sequelize/issues/3991)
 - [FIXED] Call `conformOptions` on scopes returned by functions [#3991](https://github.com/sequelize/sequelize/issues/3991)
+- [FIXED] Calling `validateIncludedElements` should not add an aliassed primary key multiple times [#4127](https://github.com/sequelize/sequelize/issues/4127)
 
 
 # 3.4.1
 # 3.4.1
 - [FIXED] Fix belongs-to-many `countAssociations` - ambigious id when through model has id
 - [FIXED] Fix belongs-to-many `countAssociations` - ambigious id when through model has id

lib/dialects/abstract/query-generator.js

@@ -1267,7 +1267,7 @@ var QueryGenerator = {
             };
             };
 
 
             if (!options.where) options.where = {};
             if (!options.where) options.where = {};
-            
+
             // Creating the as-is where for the subQuery, checks that the required association exists
             // Creating the as-is where for the subQuery, checks that the required association exists
             $query = self.selectQuery(include.model.getTableName(), {
             $query = self.selectQuery(include.model.getTableName(), {
               attributes: [association.identifierField],
               attributes: [association.identifierField],
@@ -1499,7 +1499,7 @@ var QueryGenerator = {
                    left.primaryKeyAttribute
                    left.primaryKeyAttribute
       , fieldLeft = association instanceof BelongsTo ?
       , fieldLeft = association instanceof BelongsTo ?
                    association.identifierField :
                    association.identifierField :
-                   left.rawAttributes[left.primaryKeyAttribute].field 
+                   left.rawAttributes[left.primaryKeyAttribute].field
 
 
       /* Attributes for the right side */
       /* Attributes for the right side */
       , right = association.target
       , right = association.target

lib/model.js

@@ -516,9 +516,15 @@ validateIncludedElement = function(include, tableNames, options) {
     include.originalAttributes = include.attributes.slice(0);
     include.originalAttributes = include.attributes.slice(0);
 
 
     if (include.attributes.length) {
     if (include.attributes.length) {
-      include.model.primaryKeyAttributes.forEach(function (attr) {
-        if (include.attributes.indexOf(attr) === -1) {
-          include.attributes.unshift(attr);
+      _.each(include.model.primaryKeys, function (attr, key) {
+        // Include the primary key if its not already take - take into account that the pk might be aliassed (due to a .field prop)
+        if (!_.any(include.attributes, function (includeAttr) {
+          if (attr.field !== key) {
+            return Array.isArray(includeAttr) && includeAttr[0] === attr.field && includeAttr[1] === key;
+          }
+          return includeAttr === key;
+        })) {
+          include.attributes.unshift(key);
         }
         }
       });
       });
     }
     }

test/unit/model/include.test.js

@@ -32,6 +32,12 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         title: Sequelize.STRING
         title: Sequelize.STRING
       });
       });
       this.Company = this.sequelize.define('Company', {
       this.Company = this.sequelize.define('Company', {
+        id: {
+          type: Sequelize.INTEGER,
+          primaryKey: true,
+          autoIncrement: true,
+          field: 'field_id'
+        },
         name: Sequelize.STRING
         name: Sequelize.STRING
       });
       });
 
 
@@ -41,6 +47,27 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       this.Company.Owner = this.Company.belongsTo(this.User, {as: 'Owner', foreignKey: 'ownerId'});
       this.Company.Owner = this.Company.belongsTo(this.User, {as: 'Owner', foreignKey: 'ownerId'});
     });
     });
 
 
+    describe('attributes', function () {
+      it('should not inject the aliassed PK again, if its already there', function () {
+        var options = Sequelize.Model.$validateIncludedElements({
+          model: this.User,
+          include: [
+            {
+              model: this.Company,
+              attributes: ['name']
+            }
+          ]
+        });
+
+        expect(options.include[0].attributes).to.deep.equal([['field_id', 'id'], 'name']);
+
+        options = Sequelize.Model.$validateIncludedElements(options);
+
+        // Calling validate again shouldn't add the pk again
+        expect(options.include[0].attributes).to.deep.equal([['field_id', 'id'], 'name']);
+      });
+    });
+
     describe('duplicating', function () {
     describe('duplicating', function () {
       it('should tag a hasMany association as duplicating: true if undefined', function () {
       it('should tag a hasMany association as duplicating: true if undefined', function () {
         var options = Sequelize.Model.$validateIncludedElements({
         var options = Sequelize.Model.$validateIncludedElements({
@@ -247,6 +274,6 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         expect(options.subQuery).to.equal(false);
         expect(options.subQuery).to.equal(false);
         expect(options.include[0].subQuery).to.equal(false);
         expect(options.include[0].subQuery).to.equal(false);
       });
       });
-    });  
+    });
   });
   });
-});
+});
\ No newline at end of file