Update postgres query interface (#6279)

* Fixed Issues with `createFunction` and `dropFunction` (PostgresSQL)

* line cleanup

changelog.md

+# Future
+- [FIXED] Issues with `createFunction` and `dropFunction` (PostgresSQL)
+
+
 # 4.0.0-2
 # 4.0.0-2
 - [ADDED] include now supports string as an argument (on top of model/association), string will expand into an association matched literally from Model.associations
 - [ADDED] include now supports string as an argument (on top of model/association), string will expand into an association matched literally from Model.associations
 - [FIXED] Accept dates as string while using `typeValidation` [#6453](https://github.com/sequelize/sequelize/issues/6453)
 - [FIXED] Accept dates as string while using `typeValidation` [#6453](https://github.com/sequelize/sequelize/issues/6453)

lib/dialects/postgres/query-generator.js

@@ -530,6 +530,7 @@ const QueryGenerator = {
   },
   },
 
 
   createFunction(functionName, params, returnType, language, body, options) {
   createFunction(functionName, params, returnType, language, body, options) {
+    if (!functionName || !returnType || !language || !body) throw new Error('createFunction missing some parameters. Did you pass functionName, returnType, language and body?');
 
 
     const paramList = this.expandFunctionParamList(params);
     const paramList = this.expandFunctionParamList(params);
     const indentedBody = body.replace('\n', '\n\t');
     const indentedBody = body.replace('\n', '\n\t');
@@ -544,9 +545,10 @@ const QueryGenerator = {
   },
   },
 
 
   dropFunction(functionName, params) {
   dropFunction(functionName, params) {
+    if (!functionName) throw new Error('requires functionName');
     // RESTRICT is (currently, as of 9.2) default but we'll be explicit
     // RESTRICT is (currently, as of 9.2) default but we'll be explicit
     const paramList = this.expandFunctionParamList(params);
     const paramList = this.expandFunctionParamList(params);
-    return `DROP FUNCTION${functionName}(${paramList}) RESTRICT;`;
+    return `DROP FUNCTION ${functionName}(${paramList}) RESTRICT;`;
   },
   },
 
 
   renameFunction(oldFunctionName, params, newFunctionName) {
   renameFunction(oldFunctionName, params, newFunctionName) {
@@ -575,20 +577,26 @@ const QueryGenerator = {
       throw new Error('expandFunctionParamList: function parameters array required, including an empty one for no arguments');
       throw new Error('expandFunctionParamList: function parameters array required, including an empty one for no arguments');
     }
     }
 
 
-    const paramList = Utils._.each(params, curParam => {
+    const paramList = [];
+    Utils._.each(params, curParam => {
       const paramDef = [];
       const paramDef = [];
       if (Utils._.has(curParam, 'type')) {
       if (Utils._.has(curParam, 'type')) {
         if (Utils._.has(curParam, 'direction')) { paramDef.push(curParam.direction); }
         if (Utils._.has(curParam, 'direction')) { paramDef.push(curParam.direction); }
         if (Utils._.has(curParam, 'name')) { paramDef.push(curParam.name); }
         if (Utils._.has(curParam, 'name')) { paramDef.push(curParam.name); }
         paramDef.push(curParam.type);
         paramDef.push(curParam.type);
       } else {
       } else {
-        throw new Error('createFunction called with a parameter with no type');
+        throw new Error('function or trigger used with a parameter without any type');
       }
       }
-      return paramDef.join(' ');
+
+      var joined = paramDef.join(' ');
+      if (joined) paramList.push(joined);
+
     });
     });
+
     return paramList.join(', ');
     return paramList.join(', ');
   },
   },
   
   
+
   expandOptions(options) {
   expandOptions(options) {
     return Utils._.isUndefined(options) || Utils._.isEmpty(options) ?
     return Utils._.isUndefined(options) || Utils._.isEmpty(options) ?
       '' : '\n\t' + options.join('\n\t');
       '' : '\n\t' + options.join('\n\t');

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

@@ -6,8 +6,11 @@ var chai = require('chai')
   , Support = require(__dirname + '/../../support')
   , Support = require(__dirname + '/../../support')
   , dialect = Support.getTestDialect()
   , dialect = Support.getTestDialect()
   , DataTypes = require(__dirname + '/../../../../lib/data-types')
   , DataTypes = require(__dirname + '/../../../../lib/data-types')
+  , Sequelize = require('../../../../index')
+  , Promise = Sequelize.Promise
   , _ = require('lodash');
   , _ = require('lodash');
 
 
+
 if (dialect.match(/^postgres/)) {
 if (dialect.match(/^postgres/)) {
   describe('[POSTGRES Specific] QueryInterface', function () {
   describe('[POSTGRES Specific] QueryInterface', function () {
     beforeEach(function () {
     beforeEach(function () {
@@ -15,6 +18,131 @@ if (dialect.match(/^postgres/)) {
       this.queryInterface = this.sequelize.getQueryInterface();
       this.queryInterface = this.sequelize.getQueryInterface();
     });
     });
 
 
+    describe('createFunction', function () {
+
+      beforeEach( function () {
+        //make sure we don't have a pre-existing function called create_job
+        //this is needed to cover the edge case of afterEach not getting called because of an unexpected issue or stopage with the 
+        //test suite causing a failure of afterEach's cleanup to be called.
+        return this.queryInterface.dropFunction('create_job',[{type:'varchar',name:'test'}])
+        //suppress errors here. if create_job doesn't exist thats ok.
+        .catch( err => {});
+      });
+
+      after( function () {
+        //cleanup
+        return this.queryInterface.dropFunction('create_job',[{type:'varchar',name:'test'}])
+        //suppress errors here. if create_job doesn't exist thats ok.
+        .catch( err => {});
+      });
+
+      it('creates a stored procedure', function () {
+        var body = 'return test;';
+        var options = {};
+
+        //make our call to create a function
+        return this.queryInterface.createFunction('create_job', [{type:'varchar',name:'test'}], 'varchar', 'plpgsql', body, options)
+        //validate
+        .then( () =>  {
+          return this.sequelize.query('select create_job(\'test\');', { type: this.sequelize.QueryTypes.SELECT });
+        })
+        .then( res => {
+          return expect(res[0].create_job).to.be.eql('test');
+        });
+      });
+
+      it('treats options as optional', function () {
+        var body = 'return test;';
+
+        //run with null options parameter
+        return this.queryInterface.createFunction('create_job', [{type:'varchar',name:'test'}], 'varchar', 'plpgsql', body, null)
+        //validate
+        .then( () => { 
+          return this.sequelize.query('select create_job(\'test\');', { type: this.sequelize.QueryTypes.SELECT });
+        })
+        .then( res => {
+          return expect(res[0].create_job).to.be.eql('test');
+        });
+      });
+
+      it('produces an error when missing expected parameters', function () {
+        var body = 'return 1;';
+        var options = {};
+        
+        return Promise.all([
+          //requires functionName  
+          expect( () => {
+            return this.queryInterface.createFunction(null, [{name:'test'}], 'integer', 'plpgsql', body, options);
+          }).to.throw(/createFunction missing some parameters. Did you pass functionName, returnType, language and body/)
+        
+          //requires Parameters array
+          ,expect( () => {
+            return this.queryInterface.createFunction('create_job',null, 'integer', 'plpgsql', body, options);
+          }).to.throw(/function parameters array required/)
+          
+          //requires returnType
+          ,expect( () => {
+            return this.queryInterface.createFunction('create_job', [{type:'varchar',name:'test'}], null, 'plpgsql', body, options);
+          }).to.throw(/createFunction missing some parameters. Did you pass functionName, returnType, language and body/)
+        
+          //requires type in parameter array
+          ,expect( () => {
+            return this.queryInterface.createFunction('create_job', [{name:'test'}], 'integer', 'plpgsql', body, options);
+          }).to.throw(/function or trigger used with a parameter without any type/)
+          
+          //requires language
+          ,expect( () => {
+            return this.queryInterface.createFunction('create_job', [{type:'varchar',name:'test'}], 'varchar', null, body, options);
+          }).to.throw(/createFunction missing some parameters. Did you pass functionName, returnType, language and body/)
+
+          //requires body
+          ,expect( () => {
+            return this.queryInterface.createFunction('create_job', [{type:'varchar',name:'test'}], 'varchar', 'plpgsql', null, options);
+          }).to.throw(/createFunction missing some parameters. Did you pass functionName, returnType, language and body/)
+        ]);
+      });
+    });
+
+    describe('dropFunction',function () {
+      beforeEach( function () {
+        var body = 'return test;';
+        var options = {};
+
+        //make sure we have a droptest function in place.
+        return this.queryInterface.createFunction('droptest', [{type:'varchar',name:'test'}], 'varchar', 'plpgsql', body, options)
+        //suppress errors.. this could fail if the function is already there.. thats ok. 
+        .catch( function (err) { });
+      });
+
+      it('can drop a function', function () {
+        //call drop function
+        return expect(this.queryInterface.dropFunction('droptest',[{type:'varchar',name:'test'}])
+        //now call the function we attempted to drop.. if dropFunction worked as expect it should produce an error.
+        .then( () => {
+          // call the function we attempted to drop.. if it is still there then throw an error informing that the expected behavior is not met. 
+          return this.sequelize.query('select droptest(\'test\');', { type: this.sequelize.QueryTypes.SELECT });  
+        }))
+        //test that we did get the expected error indicating that droptest was properly removed.
+        .to.be.rejectedWith(/.*function droptest.* does not exist/);
+      });
+
+      it('produces an error when missing expected parameters', function () {
+        return Promise.all([
+          expect( () => {
+            return this.queryInterface.dropFunction(); 
+          }).to.throw(/.*requires functionName/)
+          ,
+          expect( () => {
+            return this.queryInterface.dropFunction('droptest'); 
+          }).to.throw(/.*function parameters array required/)
+          ,
+          expect( () => { 
+            return this.queryInterface.dropFunction('droptest', [{name:'test'}]);
+          }).to.be.throw(/.*function or trigger used with a parameter without any type/)
+        ]);
+      });
+    });
+
     describe('indexes', function () {
     describe('indexes', function () {
       beforeEach(function () {
       beforeEach(function () {
         var self = this;
         var self = this;