fix(postgres): improve ensureEnums to support out of order enum values (#11249)

lib/dialects/postgres/query-interface.js

@@ -54,10 +54,34 @@ function ensureEnums(qi, tableName, attributes, options, model) {
     promises = [];
     promises = [];
     let enumIdx = 0;
     let enumIdx = 0;
 
 
+    // This little function allows us to re-use the same code that prepends or appends new value to enum array
+    const addEnumValue = (field, value, relativeValue, position = 'before', spliceStart = promises.length) => {
+      const valueOptions = _.clone(options);
+      valueOptions.before = null;
+      valueOptions.after = null;
+
+      switch (position) {
+        case 'after':
+          valueOptions.after = relativeValue;
+          break;
+        case 'before':
+        default:
+          valueOptions.before = relativeValue;
+          break;
+      }
+
+      promises.splice(spliceStart, 0, () => {
+        return qi.sequelize.query(qi.QueryGenerator.pgEnumAdd(
+          tableName, field, value, valueOptions
+        ), valueOptions);
+      });
+    };
+
     for (i = 0; i < keyLen; i++) {
     for (i = 0; i < keyLen; i++) {
       const attribute = attributes[keys[i]];
       const attribute = attributes[keys[i]];
       const type = attribute.type;
       const type = attribute.type;
       const enumType = type.type || type;
       const enumType = type.type || type;
+      const field = attribute.field || keys[i];
 
 
       if (
       if (
         type instanceof DataTypes.ENUM ||
         type instanceof DataTypes.ENUM ||
@@ -65,38 +89,61 @@ function ensureEnums(qi, tableName, attributes, options, model) {
       ) {
       ) {
         // If the enum type doesn't exist then create it
         // If the enum type doesn't exist then create it
         if (!results[enumIdx]) {
         if (!results[enumIdx]) {
-          sql = qi.QueryGenerator.pgEnum(tableName, attribute.field || keys[i], enumType, options);
-          promises.push(qi.sequelize.query(
-            sql,
-            Object.assign({}, options, { raw: true })
-          ));
+          promises.push(() => {
+            return qi.sequelize.query(qi.QueryGenerator.pgEnum(tableName, field, enumType, options), Object.assign({}, options, { raw: true }));
+          });
         } else if (!!results[enumIdx] && !!model) {
         } else if (!!results[enumIdx] && !!model) {
           const enumVals = qi.QueryGenerator.fromArray(results[enumIdx].enum_value);
           const enumVals = qi.QueryGenerator.fromArray(results[enumIdx].enum_value);
           const vals = enumType.values;
           const vals = enumType.values;
 
 
-          vals.forEach((value, idx) => {
-            // reset out after/before options since it's for every enum value
-            const valueOptions = _.clone(options);
-            valueOptions.before = null;
-            valueOptions.after = null;
+          // Going through already existing values allows us to make queries that depend on those values
+          // We will prepend all new values between the old ones, but keep in mind - we can't change order of already existing values
+          // Then we append the rest of new values AFTER the latest already existing value
+          // E.g.: [1,2] -> [0,2,1] ==> [1,0,2]
+          // E.g.: [1,2,3] -> [2,1,3,4] ==> [1,2,3,4]
+          // E.g.: [1] -> [0,2,3] ==> [1,0,2,3]
+          let lastOldEnumValue;
+          let rightestPosition = -1;
+          for (let oldIndex = 0; oldIndex < enumVals.length; oldIndex++) {
+            const enumVal = enumVals[oldIndex];
+            const newIdx = vals.indexOf(enumVal);
+            lastOldEnumValue = enumVal;
+
+            if (newIdx === -1) {
+              continue;
+            }
 
 
-            if (!enumVals.includes(value)) {
-              if (vals[idx + 1]) {
-                valueOptions.before = vals[idx + 1];
-              }
-              else if (vals[idx - 1]) {
-                valueOptions.after = vals[idx - 1];
+            const newValuesBefore = vals.slice(0, newIdx);
+            const promisesLength = promises.length;
+            // we go in reverse order so we could stop when we meet old value
+            for (let reverseIdx = newValuesBefore.length - 1; reverseIdx >= 0; reverseIdx--) {
+              if (~enumVals.indexOf(newValuesBefore[reverseIdx])) {
+                break;
               }
               }
-              valueOptions.supportsSearchPath = false;
-              promises.push(qi.sequelize.query(qi.QueryGenerator.pgEnumAdd(tableName, attribute.field || keys[i], value, valueOptions), valueOptions));
+
+              addEnumValue(field, newValuesBefore[reverseIdx], lastOldEnumValue, 'before', promisesLength);
             }
             }
-          });
+
+            // we detect the most 'right' position of old value in new enum array so we can append new values to it
+            if (newIdx > rightestPosition) {
+              rightestPosition = newIdx;
+            }
+          }
+
+          if (lastOldEnumValue && rightestPosition < vals.length - 1) {
+            const remainingEnumValues = vals.slice(rightestPosition + 1);
+            for (let reverseIdx = remainingEnumValues.length - 1; reverseIdx >= 0; reverseIdx--) {
+              addEnumValue(field, remainingEnumValues[reverseIdx], lastOldEnumValue, 'after');
+            }
+          }
+
           enumIdx++;
           enumIdx++;
         }
         }
       }
       }
     }
     }
 
 
-    return Promise.all(promises)
+    return promises
+      .reduce((promise, asyncFunction) => promise.then(asyncFunction), Promise.resolve())
       .tap(() => {
       .tap(() => {
         // If ENUM processed, then refresh OIDs
         // If ENUM processed, then refresh OIDs
         if (promises.length) {
         if (promises.length) {

test/integration/dialects/postgres/dao.test.js

@@ -367,6 +367,25 @@ if (dialect.match(/^postgres/)) {
         });
         });
       });
       });
 
 
+      it('should be able to add multiple values with different order', function() {
+        let User = this.sequelize.define('UserEnums', {
+          priority: DataTypes.ENUM('1', '2', '6')
+        });
+
+        return User.sync({ force: true }).then(() => {
+          User = this.sequelize.define('UserEnums', {
+            priority: DataTypes.ENUM('0', '1', '2', '3', '4', '5', '6', '7')
+          });
+
+          return User.sync();
+        }).then(() => {
+          return this.sequelize.getQueryInterface().pgListEnums(User.getTableName());
+        }).then(enums => {
+          expect(enums).to.have.length(1);
+          expect(enums[0].enum_value).to.equal('{0,1,2,3,4,5,6,7}');
+        });
+      });
+
       describe('ARRAY(ENUM)', () => {
       describe('ARRAY(ENUM)', () => {
         it('should be able to ignore enum types that already exist', function() {
         it('should be able to ignore enum types that already exist', function() {
           const User = this.sequelize.define('UserEnums', {
           const User = this.sequelize.define('UserEnums', {

test/integration/query-interface/createTable.test.js

@@ -92,60 +92,93 @@ describe(Support.getTestDialectTeaser('QueryInterface'), () => {
       });
       });
     });
     });
 
 
-    it('should work with enums (1)', function() {
-      return this.queryInterface.createTable('SomeTable', {
-        someEnum: DataTypes.ENUM('value1', 'value2', 'value3')
+    it('should work with schemas', function() {
+      return this.sequelize.createSchema('hero').then(() => {
+        return this.queryInterface.createTable('User', {
+          name: {
+            type: DataTypes.STRING
+          }
+        }, {
+          schema: 'hero'
+        });
       });
       });
     });
     });
 
 
-    it('should work with enums (2)', function() {
-      return this.queryInterface.createTable('SomeTable', {
-        someEnum: {
-          type: DataTypes.ENUM,
-          values: ['value1', 'value2', 'value3']
-        }
+    describe('enums', () => {
+      it('should work with enums (1)', function() {
+        return this.queryInterface.createTable('SomeTable', {
+          someEnum: DataTypes.ENUM('value1', 'value2', 'value3')
+        }).then(() => {
+          return this.queryInterface.describeTable('SomeTable');
+        }).then(table => {
+          if (dialect.includes('postgres')) {
+            expect(table.someEnum.special).to.deep.equal(['value1', 'value2', 'value3']);
+          }
+        });
       });
       });
-    });
 
 
-    it('should work with enums (3)', function() {
-      return this.queryInterface.createTable('SomeTable', {
-        someEnum: {
-          type: DataTypes.ENUM,
-          values: ['value1', 'value2', 'value3'],
-          field: 'otherName'
-        }
+      it('should work with enums (2)', function() {
+        return this.queryInterface.createTable('SomeTable', {
+          someEnum: {
+            type: DataTypes.ENUM,
+            values: ['value1', 'value2', 'value3']
+          }
+        }).then(() => {
+          return this.queryInterface.describeTable('SomeTable');
+        }).then(table => {
+          if (dialect.includes('postgres')) {
+            expect(table.someEnum.special).to.deep.equal(['value1', 'value2', 'value3']);
+          }
+        });
       });
       });
-    });
 
 
-    it('should work with enums (4)', function() {
-      return this.queryInterface.createSchema('archive').then(() => {
+      it('should work with enums (3)', function() {
         return this.queryInterface.createTable('SomeTable', {
         return this.queryInterface.createTable('SomeTable', {
           someEnum: {
           someEnum: {
             type: DataTypes.ENUM,
             type: DataTypes.ENUM,
             values: ['value1', 'value2', 'value3'],
             values: ['value1', 'value2', 'value3'],
             field: 'otherName'
             field: 'otherName'
           }
           }
-        }, { schema: 'archive' });
+        }).then(() => {
+          return this.queryInterface.describeTable('SomeTable');
+        }).then(table => {
+          if (dialect.includes('postgres')) {
+            expect(table.otherName.special).to.deep.equal(['value1', 'value2', 'value3']);
+          }
+        });
       });
       });
-    });
 
 
-    it('should work with enums (5)', function() {
-      return this.queryInterface.createTable('SomeTable', {
-        someEnum: {
-          type: DataTypes.ENUM(['COMMENT']),
-          comment: 'special enum col'
-        }
+      it('should work with enums (4)', function() {
+        return this.queryInterface.createSchema('archive').then(() => {
+          return this.queryInterface.createTable('SomeTable', {
+            someEnum: {
+              type: DataTypes.ENUM,
+              values: ['value1', 'value2', 'value3'],
+              field: 'otherName'
+            }
+          }, { schema: 'archive' });
+        }).then(() => {
+          return this.queryInterface.describeTable('SomeTable', { schema: 'archive' });
+        }).then(table => {
+          if (dialect.includes('postgres')) {
+            expect(table.otherName.special).to.deep.equal(['value1', 'value2', 'value3']);
+          }
+        });
       });
       });
-    });
 
 
-    it('should work with schemas', function() {
-      return this.sequelize.createSchema('hero').then(() => {
-        return this.queryInterface.createTable('User', {
-          name: {
-            type: DataTypes.STRING
+      it('should work with enums (5)', function() {
+        return this.queryInterface.createTable('SomeTable', {
+          someEnum: {
+            type: DataTypes.ENUM(['COMMENT']),
+            comment: 'special enum col'
+          }
+        }).then(() => {
+          return this.queryInterface.describeTable('SomeTable');
+        }).then(table => {
+          if (dialect.includes('postgres')) {
+            expect(table.someEnum.special).to.deep.equal(['COMMENT']);
+            expect(table.someEnum.comment).to.equal('special enum col');
           }
           }
-        }, {
-          schema: 'hero'
         });
         });
       });
       });
     });
     });