bug(schema+enum) Be more consistent when naming enums, especially in the presenc…

…e of schemas. Closes #3171, Closes #3563

lib/dialects/postgres/query-generator.js

@@ -8,7 +8,8 @@ var Utils = require('../../utils')
   , DataTypes = require('../../data-types')
   , SqlString = require('../../sql-string')
   , AbstractQueryGenerator = require('../abstract/query-generator')
-  , primaryKeys = {};
+  , primaryKeys = {}
+  , _ = require('lodash');
 
 var QueryGenerator = {
   options: {},
@@ -756,28 +757,36 @@ var QueryGenerator = {
     }).join(' OR ');
   },
 
-  pgListEnums: function(tableName, attrName, options) {
-    if (arguments.length === 1) {
-      options = tableName;
-      tableName = null;
+  pgEnumName: function (tableName, attr, options) {
+    options = options || {};
+
+    var enumName = '"enum_' + (_.isPlainObject(tableName) ? tableName.table : tableName) + '_' + attr + '"';
+
+    if (options.schema !== false && tableName && tableName.schema) {
+      enumName = '"' + tableName.schema + '"' + tableName.delimiter + enumName;
     }
 
-    var enumName = '';
+    return enumName;
+  },
+
+  pgListEnums: function(tableName, attrName) {
+    var enumName = ''
+      , schema = tableName && tableName.schema ? tableName.schema : 'public';
 
     if (!!tableName && !!attrName) {
-      enumName = ' AND t.typname=' + this.escape('enum_' + tableName + '_' + attrName) + ' ';
+      enumName = ' AND t.typname=' + this.pgEnumName(tableName, attrName, { schema: false }).replace(/"/g, "'");
     }
 
     var query = 'SELECT t.typname enum_name, array_agg(e.enumlabel) enum_value FROM pg_type t ' +
       'JOIN pg_enum e ON t.oid = e.enumtypid ' +
       'JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace ' +
-      "WHERE n.nspname = 'public' " + enumName + ' GROUP BY 1';
+      "WHERE n.nspname = '" + schema + "'" + enumName + ' GROUP BY 1';
 
     return query;
   },
 
   pgEnum: function(tableName, attr, dataType, options) {
-    var enumName = this.pgEscapeAndQuote('enum_' + tableName + '_' + attr)
+    var enumName = this.pgEnumName(tableName, attr)
       , values;
 
     if (dataType.values) {
@@ -786,7 +795,7 @@ var QueryGenerator = {
       values = dataType.toString().match(/^ENUM\(.+\)/)[0];
     }
 
-    var sql = 'CREATE TYPE ' + enumName + ' AS ' + values + '; ';
+    var sql = 'CREATE TYPE ' + enumName + ' AS ' + values + ';';
     if (!!options && options.force === true) {
       sql = this.pgEnumDrop(tableName, attr) + sql;
     }
@@ -794,8 +803,8 @@ var QueryGenerator = {
   },
 
   pgEnumAdd: function(tableName, attr, value, options) {
-    var enumName = this.pgEscapeAndQuote('enum_' + tableName + '_' + attr);
-    var sql = 'ALTER TYPE ' + enumName + ' ADD VALUE ' + this.escape(value);
+    var enumName = this.pgEnumName(tableName, attr)
+      , sql = 'ALTER TYPE ' + enumName + ' ADD VALUE ' + this.escape(value);
 
     if (!!options.before) {
       sql += ' BEFORE ' + this.escape(options.before);
@@ -808,7 +817,7 @@ var QueryGenerator = {
   },
 
   pgEnumDrop: function(tableName, attr, enumName) {
-    enumName = enumName || this.pgEscapeAndQuote('enum_' + tableName + '_' + attr);
+    enumName = enumName || this.pgEnumName(tableName, attr);
     return 'DROP TYPE IF EXISTS ' + enumName + '; ';
   },
 
@@ -852,7 +861,7 @@ var QueryGenerator = {
     }
 
     if (dataType.match(/^ENUM\(/)) {
-      dataType = dataType.replace(/^ENUM\(.+\)/, this.pgEscapeAndQuote('enum_' + tableName + '_' + attr));
+      dataType = dataType.replace(/^ENUM\(.+\)/, this.pgEnumName(tableName, attr));
     }
 
     return dataType;

lib/model.js

@@ -936,7 +936,7 @@ Model.prototype.sync = function(options) {
       return self.drop(options);
     }
   }).then(function () {
-    return self.QueryInterface.createTable(self.getTableName(options), attributes, options);
+    return self.QueryInterface.createTable(self.getTableName(options), attributes, options, self);
   }).then(function () {
     return self.QueryInterface.showIndex(self.getTableName(options), options);
   }).then(function (indexes) {

lib/query-interface.js

@@ -68,7 +68,7 @@ QueryInterface.prototype.databaseVersion = function(options) {
   }, options));
 };
 
-QueryInterface.prototype.createTable = function(tableName, attributes, options) {
+QueryInterface.prototype.createTable = function(tableName, attributes, options, model) {
   var keys = Object.keys(attributes)
     , keyLen = keys.length
     , self = this
@@ -108,14 +108,11 @@ QueryInterface.prototype.createTable = function(tableName, attributes, options)
 
   // Postgres requires a special SQL command for enums
   if (self.sequelize.options.dialect === 'postgres') {
-    var promises = []
-      // For backwards-compatibility, public schemas don't need to
-      // explicitly state their schema when creating a new enum type
-      , getTableName = (!options || !options.schema || options.schema === 'public' ? '' : options.schema + '_') + tableName;
+    var promises = [];
 
     for (i = 0; i < keyLen; i++) {
       if (attributes[keys[i]].type instanceof DataTypes.ENUM) {
-        sql = self.QueryGenerator.pgListEnums(getTableName, attributes[keys[i]].field || keys[i], options);
+        sql = self.QueryGenerator.pgListEnums(tableName, attributes[keys[i]].field || keys[i]);
         promises.push(self.sequelize.query(
           sql, { plain: true, raw: true, type: QueryTypes.SELECT, logging: options.logging, transaction: options.transaction }
         ));
@@ -124,22 +121,19 @@ QueryInterface.prototype.createTable = function(tableName, attributes, options)
 
     return Promise.all(promises).then(function(results) {
       var promises = []
-        , instanceTable = self.sequelize.modelManager.models.filter(function(instance) { return instance.tableName === tableName; })
         , enumIdx = 0;
 
-      instanceTable = instanceTable.length > 0 ? instanceTable[0] : null;
-
       for (i = 0; i < keyLen; i++) {
         if (attributes[keys[i]].type instanceof DataTypes.ENUM) {
           // If the enum type doesn't exist then create it
           if (!results[enumIdx]) {
-            sql = self.QueryGenerator.pgEnum(getTableName, attributes[keys[i]].field || keys[i], attributes[keys[i]], options);
+            sql = self.QueryGenerator.pgEnum(tableName, attributes[keys[i]].field || keys[i], attributes[keys[i]], options);
             promises.push(self.sequelize.query(
               sql, { raw: true, logging: options.logging, transaction: options.transaction }
             ));
-          } else if (!!results[enumIdx] && !!instanceTable) {
+          } else if (!!results[enumIdx] && !!model) {
             var enumVals = self.QueryGenerator.fromArray(results[enumIdx].enum_value)
-              , vals = instanceTable.rawAttributes[keys[i]].values;
+              , vals = model.rawAttributes[keys[i]].values;
 
             vals.forEach(function(value, idx) {
               // reset out after/before options since it's for every enum value
@@ -153,7 +147,7 @@ QueryInterface.prototype.createTable = function(tableName, attributes, options)
                 else if (!!vals[idx - 1]) {
                   options.after = vals[idx - 1];
                 }
-                promises.push(self.sequelize.query(self.QueryGenerator.pgEnumAdd(getTableName, keys[i], value, options), options));
+                promises.push(self.sequelize.query(self.QueryGenerator.pgEnumAdd(tableName, keys[i], value, options), options));
               }
             });
             enumIdx++;

test/integration/dialects/postgres/query-generator.test.js

@@ -204,7 +204,7 @@ if (dialect.match(/^postgres/)) {
         },
         {
           arguments: ['myTable', {title: 'ENUM("A", "B", "C")', name: 'VARCHAR(255)'}],
-          expectation: 'CREATE TABLE IF NOT EXISTS myTable (title enum_myTable_title, name VARCHAR(255));',
+          expectation: 'CREATE TABLE IF NOT EXISTS myTable (title "enum_myTable_title", name VARCHAR(255));',
           context: {options: {quoteIdentifiers: false}}
         },
         {

test/unit/sql/create-table.test.js

+'use strict';
+
+/* jshint -W110 */
+var Support   = require(__dirname + '/../support')
+  , DataTypes = require(__dirname + '/../../../lib/data-types')
+  , expectsql = Support.expectsql
+  , current   = Support.sequelize
+  , sql       = current.dialect.QueryGenerator;
+
+
+describe(Support.getTestDialectTeaser('SQL'), function() {
+  describe.only('createTable', function () {
+    describe('with enums', function () {
+      var FooUser = current.define('user', {
+        mood: DataTypes.ENUM('happy', 'sad')
+      },{
+        schema: 'foo',
+        timestamps: false
+      });
+
+      it('references enum in the right schema #3171', function () {
+        expectsql(sql.createTableQuery(FooUser.getTableName(), sql.attributesToSQL(FooUser.rawAttributes), { }), {
+          sqlite: 'CREATE TABLE IF NOT EXISTS `foo.users` (`id` INTEGER PRIMARY KEY AUTOINCREMENT, `mood` TEXT);',
+          postgres: 'CREATE TABLE IF NOT EXISTS "foo"."users" ("id"   SERIAL , "mood" "foo"."enum_users_mood", PRIMARY KEY ("id"));',
+          mysql: "CREATE TABLE IF NOT EXISTS `foo.users` (`id` INTEGER NOT NULL auto_increment , `mood` ENUM('happy', 'sad'), PRIMARY KEY (`id`)) ENGINE=InnoDB;",
+          mssql: "IF OBJECT_ID('[foo].[users]', 'U') IS NULL CREATE TABLE [foo].[users] ([id] INTEGER NOT NULL IDENTITY(1,1) , [mood] VARCHAR(255) CHECK (mood IN('happy', 'sad')), PRIMARY KEY ([id]));"
+        });
+      });
+    });
+  });
+});
+

test/unit/sql/enum.test.js

+'use strict';
+
+/* jshint -W110 */
+var Support   = require(__dirname + '/../support')
+  , DataTypes = require(__dirname + '/../../../lib/data-types')
+  , expectsql = Support.expectsql
+  , current   = Support.sequelize
+  , sql       = current.dialect.QueryGenerator;
+
+
+describe(Support.getTestDialectTeaser('SQL'), function() {
+  describe('enum', function () {
+    if (Support.getTestDialect() === 'postgres') {
+      var FooUser = current.define('user', {
+        mood: DataTypes.ENUM('happy', 'sad')
+      },{
+        schema: 'foo'
+      });
+
+      var PublicUser = current.define('user', {
+        mood: {
+          type: DataTypes.ENUM('happy', 'sad'),
+          field: 'theirMood'
+        }
+      });
+
+      describe('pgEnum', function () {
+        it('uses schema #3171', function () {
+          expectsql(sql.pgEnum(FooUser.getTableName(), 'mood', FooUser.rawAttributes.mood.type), {
+            postgres: 'CREATE TYPE "foo"."enum_users_mood" AS ENUM(\'happy\', \'sad\');'
+          });
+        });
+
+        it('does not add schema when public', function () {
+          expectsql(sql.pgEnum(PublicUser.getTableName(), 'theirMood', PublicUser.rawAttributes.mood.type), {
+            postgres: 'CREATE TYPE "enum_users_theirMood" AS ENUM(\'happy\', \'sad\');'
+          });
+        });
+      });
+
+      describe('pgListEnums', function () {
+        it('works with schema #3563', function () {
+           expectsql(sql.pgListEnums(FooUser.getTableName(), 'mood'), {
+            postgres: 'SELECT t.typname enum_name, array_agg(e.enumlabel) enum_value FROM pg_type t JOIN pg_enum e ON t.oid = e.enumtypid JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace WHERE n.nspname = \'foo\' AND t.typname=\'enum_users_mood\' GROUP BY 1'
+          });
+        });
+
+        it('uses the default schema if options given', function () {
+            expectsql(sql.pgListEnums(), {
+            postgres: 'SELECT t.typname enum_name, array_agg(e.enumlabel) enum_value FROM pg_type t JOIN pg_enum e ON t.oid = e.enumtypid JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace WHERE n.nspname = \'public\' GROUP BY 1'
+          });
+        });
+      });
+    }
+  });
+});
+