7568 remove raw where (#7569)

* Removes support for $raw

* Removes interpretation of where array as a literal with replacements.

* Updates documentation to remove where object with literal replacements.

* Fixes some of the integration tests.

* Fixes integration tests and encapsulate subquery join where in a plain object.

* Adds unit tests for nested arrays in where object

* This should cover the rest of the diff.

* [ci skip] add BC break note to changelog

* Fixes changelog typo and removed the ternary because I edit in an abstract fashion.

changelog.md

@@ -68,6 +68,7 @@
 - [REMOVED] Removes support for `{raw: 'injection goes here'}` for order and group. [#7188](https://github.com/sequelize/sequelize/issues/7188)
 - [FIXED] `showIndex` breaks with newline characters [#7492](https://github.com/sequelize/sequelize/pull/7492)
 - [FIXED] Update or soft delete breaks when querying on `JSON/JSONB` [#7376](https://github.com/sequelize/sequelize/issues/7376) [#7400](https://github.com/sequelize/sequelize/issues/7400) [#7444](https://github.com/sequelize/sequelize/issues/7444)
+- [REMOVED] Removes support for interpretation of raw properties and values in the where object. [#7568](https://github.com/sequelize/sequelize/issues/7568)
 - [FIXED] Upsert now updates all changed fields by default
 
 ## BC breaks:
@@ -77,6 +78,7 @@
 - `options.order` Now only excepts values with type of array or Sequelize method.  Support for string values (ie `{order: 'name DESC'}`) has been deprecated.
 - `DataTypes.DATE` now uses `DATETIMEOFFSET` instead of `DATETIME2` sql datatype in case of MSSQL to record timezone [#5403](https://github.com/sequelize/sequelize/issues/5403)
 - `DataTypes.DECIMAL` returns string for MySQL and Postgres
+- `{ where: { $raw: "foo = 'bar'" } }` and `{ where: ['foo = ?', ['bar'] }` are no longer suported. Use `.literal` instead
 
 # 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

docs/models-usage.md

@@ -162,11 +162,6 @@ Project.findAll({ where: { name: 'A Project' } }).then(function(projects) {
   // projects will be an array of Project instances with the specified name
 })
 
-// search with string replacements
-Project.findAll({ where: ["id > ?", 25] }).then(function(projects) {
-  // projects will be an array of Projects having a greater id than 25
-})
-
 // search within a specific range
 Project.findAll({ where: { id: [1,2,3] } }).then(function(projects) {
   // projects will be an array of Projects having the id 1, 2 or 3
@@ -346,7 +341,7 @@ Project.count().then(function(c) {
   console.log("There are " + c + " projects!")
 })
 
-Project.count({ where: ["id > ?", 25] }).then(function(c) {
+Project.count({ where: {'id': {$gt: 25}} }).then(function(c) {
   console.log("There are " + c + " projects with an id greater than 25.")
 })
 ```

lib/dialects/abstract/query-generator.js

@@ -1295,7 +1295,7 @@ const QueryGenerator = {
         const associationWhere = {};
 
         associationWhere[association.identifierField] = {
-          $raw: `${this.quoteTable(parentTableName.internalAs)}.${this.quoteIdentifier(association.sourceKeyField || association.source.primaryKeyField)}`
+          $eq: this.sequelize.literal(`${this.quoteTable(parentTableName.internalAs)}.${this.quoteIdentifier(association.sourceKeyField || association.source.primaryKeyField)}`)
         };
 
         if (!topLevelInfo.options.where) {
@@ -1903,7 +1903,7 @@ const QueryGenerator = {
     }
 
     if (_.isString(where)) {
-      throw new Error('where: "raw query" has been removed, please use where ["raw query", [replacements]]');
+      throw new Error('Support for `{where: \'raw query\'}` has been removed.');
     }
 
     const items = [];
@@ -2026,7 +2026,7 @@ const QueryGenerator = {
       if (Utils.canTreatArrayAsAnd(value)) {
         key = '$and';
       } else {
-        return Utils.format(value, this.dialect);
+        throw new Error('Support for literal replacements in the `where` object has been removed.');
       }
     }
     // OR/AND/NOT grouping logic
@@ -2217,7 +2217,7 @@ const QueryGenerator = {
 
       value = (value.$between || value.$notBetween).map(item => this.escape(item)).join(' AND ');
     } else if (value && value.$raw) {
-      value = value.$raw;
+      throw new Error('The `$raw` where property is no longer supported.  Use `sequelize.literal` instead.');
     } else if (value && value.$col) {
       value = value.$col.split('.');
 
@@ -2359,12 +2359,12 @@ const QueryGenerator = {
     } else if (Buffer.isBuffer(smth)) {
       result = this.escape(smth);
     } else if (Array.isArray(smth)) {
-      if (smth.length === 0) return '1=1';
+      if (smth.length === 0 || smth.length > 0 && smth[0].length === 0) return '1=1';
       if (Utils.canTreatArrayAsAnd(smth)) {
         const _smth = { $and: smth };
         result = this.getWhereConditions(_smth, tableName, factory, options, prepend);
       } else {
-        result = Utils.format(smth, this.dialect);
+        throw new Error('Support for literal replacements in the `where` object has been removed.');
       }
     } else if (smth === null) {
       return this.whereItemsQuery(smth, {

test/integration/associations/belongs-to-many.test.js

@@ -141,7 +141,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), () => {
 
     it('only gets objects that fulfill options with a formatted value', function() {
       return this.User.find({where: {username: 'John'}}).then((john) => {
-        return john.getTasks({where: ['active = ?', true]});
+        return john.getTasks({where: {active: true}});
       }).then((tasks) => {
         expect(tasks).to.have.length(1);
       });

test/integration/associations/belongs-to.test.js

@@ -112,7 +112,7 @@ describe(Support.getTestDialectTeaser('BelongsTo'), () => {
         ]);
       }).spread((userA, userB, task) => {
         return task.setUserXYZ(userA).then(() => {
-          return task.getUserXYZ({where: ['gender = ?', 'female']});
+          return task.getUserXYZ({where: {gender: 'female'}});
         });
       }).then((user) => {
         expect(user).to.be.null;

test/integration/associations/has-many.test.js

@@ -876,7 +876,7 @@ describe(Support.getTestDialectTeaser('HasMany'), () => {
           this.article = article;
           return article.setLabels([label1, label2]);
         }).then(function() {
-          return this.article.getLabels({where: ['until > ?', moment('2014-01-02').toDate()]});
+          return this.article.getLabels({where: {until: {$gt: moment('2014-01-02').toDate()}}});
         }).then((labels) => {
           expect(labels).to.be.instanceof(Array);
           expect(labels).to.have.length(1);

test/integration/associations/has-one.test.js

@@ -108,7 +108,7 @@ describe(Support.getTestDialectTeaser('HasOne'), () => {
           return User.create({ username: 'foo' }).then((user) => {
             return Task.create({ title: 'task', status: 'inactive' }).then((task) => {
               return user.setTaskXYZ(task).then(() => {
-                return user.getTaskXYZ({where: ['status = ?', 'active']}).then((task) => {
+                return user.getTaskXYZ({where: {status: 'active'}}).then((task) => {
                   expect(task).to.be.null;
                 });
               });

test/integration/instance/update.test.js

@@ -430,7 +430,7 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
             expect(download.finishedAt).to.not.be.ok;
 
             return Download.findAll({
-              where: dialect === 'postgres' || dialect === 'mssql' ? ['"finishedAt" IS NULL'] : ['`finishedAt` IS NULL']
+              where: {finishedAt: null}
             }).then((downloads) => {
               downloads.forEach((download) => {
                 expect(download.startedAt instanceof Date).to.be.true;

test/integration/model.test.js

@@ -1772,7 +1772,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       const self = this;
       return this.User.create({username: 'user1'}).then(() => {
         return self.User.create({username: 'foo'}).then(() => {
-          return self.User.count({where: ["username LIKE '%us%'"]}).then((count) => {
+          return self.User.count({where: {username: {$like: '%us%'}}}).then((count) => {
             expect(count).to.equal(1);
           });
         });
@@ -2545,11 +2545,15 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         });
     });
 
+    it('should fail when array contains strings', function() {
+      return expect(this.User.findAll({
+        where: ['this is a mistake', ['dont do it!']]
+      })).to.eventually.be.rejectedWith(Error, 'Support for literal replacements in the `where` object has been removed.');
+    });
+
     it('should not fail with an include', function() {
       return this.User.findAll({
-        where: [
-          this.sequelize.queryInterface.QueryGenerator.quoteIdentifiers('Projects.title') + ' = ' + this.sequelize.queryInterface.QueryGenerator.escape('republic')
-        ],
+        where: this.sequelize.literal(this.sequelize.queryInterface.QueryGenerator.quoteIdentifiers('Projects.title') + ' = ' + this.sequelize.queryInterface.QueryGenerator.escape('republic')),
         include: [
           {model: this.Project}
         ]
@@ -2566,9 +2570,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       }
       return this.User.findAll({
         paranoid: false,
-        where: [
-          tableName + this.sequelize.queryInterface.QueryGenerator.quoteIdentifier('deletedAt') + ' IS NOT NULL '
-        ],
+        where: this.sequelize.literal(tableName + this.sequelize.queryInterface.QueryGenerator.quoteIdentifier('deletedAt') + ' IS NOT NULL '),
         include: [
           {model: this.Project}
         ]

test/integration/model/find.test.js

@@ -110,10 +110,10 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       it('treats questionmarks in an array', function() {
         let test = false;
         return this.UserPrimary.findOne({
-          where: ['specialkey = ?', 'awesome'],
+          where: {'specialkey': 'awesome'},
           logging(sql) {
             test = true;
-            expect(sql).to.match(/WHERE specialkey = N?'awesome'/);
+            expect(sql).to.match(/WHERE ["|`|\[]UserPrimary["|`|\]]\.["|`|\[]specialkey["|`|\]] = N?'awesome'/);
           }
         }).then(() => {
           expect(test).to.be.true;
@@ -549,7 +549,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
             });
           });
 
-          it(`throws an error indicating an incorrect alias was entered if an association 
+          it(`throws an error indicating an incorrect alias was entered if an association
               and alias exist but the alias doesn't match`, function() {
             const self = this;
             return self.Worker.findOne({ include: [{ model: self.Task, as: 'Work' }] }).catch ((err) => {
@@ -711,7 +711,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
             });
           });
 
-          it(`throws an error indicating an incorrect alias was entered if an association 
+          it(`throws an error indicating an incorrect alias was entered if an association
               and alias exist but the alias doesn't match`, function() {
             const self = this;
             return self.Worker.findOne({ include: [{ model: self.Task, as: 'Work' }] }).catch ((err) => {

test/integration/model/findAll.test.js

@@ -182,17 +182,16 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       });
 
       it('should be able to handle false/true values just fine...', function() {
-        const User = this.User,
-          escapeChar = dialect === 'postgres' || dialect === 'mssql' ? '"' : '`';
+        const User = this.User;
 
         return User.bulkCreate([
           {username: 'boo5', aBool: false},
           {username: 'boo6', aBool: true}
         ]).then(() => {
-          return User.findAll({where: [escapeChar + 'aBool' + escapeChar + ' = ?', false]}).then((users) => {
+          return User.findAll({where: {aBool: false}}).then((users) => {
             expect(users).to.have.length(1);
             expect(users[0].username).to.equal('boo5');
-            return User.findAll({where: [escapeChar + 'aBool' + escapeChar + ' = ?', true]}).then((_users) => {
+            return User.findAll({where: {aBool: true}}).then((_users) => {
               expect(_users).to.have.length(1);
               expect(_users[0].username).to.equal('boo6');
             });
@@ -202,7 +201,6 @@ describe(Support.getTestDialectTeaser('Model'), () => {
 
       it('should be able to handle false/true values through associations as well...', function() {
         const User = this.User,
-          escapeChar = dialect === 'postgres' || dialect === 'mssql' ? '"' : '`',
           Passports = this.sequelize.define('Passports', {
             isActive: Sequelize.BOOLEAN
           });
@@ -226,8 +224,8 @@ describe(Support.getTestDialectTeaser('Model'), () => {
                       return User.findById(2).then((_user) => {
                         return Passports.findById(2).then((_passport) => {
                           return _user.setPassports([_passport]).then(() => {
-                            return _user.getPassports({where: [escapeChar + 'isActive' + escapeChar + ' = ?', false]}).then((theFalsePassport) => {
-                              return user.getPassports({where: [escapeChar + 'isActive' + escapeChar + ' = ?', true]}).then((theTruePassport) => {
+                            return _user.getPassports({where: {isActive: false}}).then((theFalsePassport) => {
+                              return user.getPassports({where: {isActive: true}}).then((theTruePassport) => {
                                 expect(theFalsePassport).to.have.length(1);
                                 expect(theFalsePassport[0].isActive).to.be.false;
                                 expect(theTruePassport).to.have.length(1);
@@ -1204,9 +1202,9 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         });
       });
 
-      it('can also handle array notation', function() {
+      it('can also handle object notation', function() {
         const self = this;
-        return this.User.findAll({where: ['id = ?', this.users[1].id]}).then((users) => {
+        return this.User.findAll({where: {id: this.users[1].id}}).then((users) => {
           expect(users.length).to.equal(1);
           expect(users[0].id).to.equal(self.users[1].id);
         });
@@ -1335,16 +1333,16 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       });
     }
 
-    it('handles where clause [only]', function() {
-      return this.User.findAndCountAll({where: ['id != ' + this.users[0].id]}).then((info) => {
+    it('handles where clause {only}', function() {
+      return this.User.findAndCountAll({where: {id: {$ne: this.users[0].id}}}).then((info) => {
         expect(info.count).to.equal(2);
         expect(Array.isArray(info.rows)).to.be.ok;
         expect(info.rows.length).to.equal(2);
       });
     });
 
-    it('handles where clause with ordering [only]', function() {
-      return this.User.findAndCountAll({where: ['id != ' + this.users[0].id], order: [['id', 'ASC']]}).then((info) => {
+    it('handles where clause with ordering {only}', function() {
+      return this.User.findAndCountAll({where: {id: {$ne: this.users[0].id}}, order: [['id', 'ASC']]}).then((info) => {
         expect(info.count).to.equal(2);
         expect(Array.isArray(info.rows)).to.be.ok;
         expect(info.rows.length).to.equal(2);
@@ -1422,7 +1420,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
     });
 
     it('handles attributes', function() {
-      return this.User.findAndCountAll({where: ['id != ' + this.users[0].id], attributes: ['data']}).then((info) => {
+      return this.User.findAndCountAll({where: {id: {$ne: this.users[0].id}}, attributes: ['data']}).then((info) => {
         expect(info.count).to.equal(2);
         expect(Array.isArray(info.rows)).to.be.ok;
         expect(info.rows.length).to.equal(2);

test/unit/dialects/mysql/query-generator.test.js

@@ -181,10 +181,6 @@ if (dialect === 'mysql') {
           expectation: 'SELECT count(*) AS `count` FROM `foo`;',
           context: QueryGenerator
         }, {
-          arguments: ['myTable', {where: ["foo='bar'"]}],
-          expectation: "SELECT * FROM `myTable` WHERE foo='bar';",
-          context: QueryGenerator
-        }, {
           arguments: ['myTable', {order: ['id']}],
           expectation: 'SELECT * FROM `myTable` ORDER BY `id`;',
           context: QueryGenerator
@@ -293,18 +289,6 @@ if (dialect === 'mysql') {
           expectation: 'SELECT * FROM `myTable` GROUP BY name ORDER BY `id` DESC;',
           context: QueryGenerator
         }, {
-          title: 'HAVING clause works with string replacements',
-          arguments: ['myTable', function(sequelize) {
-            return {
-              attributes: ['*', [sequelize.fn('YEAR', sequelize.col('createdAt')), 'creationYear']],
-              group: ['creationYear', 'title'],
-              having: ['creationYear > ?', 2002]
-            };
-          }],
-          expectation: 'SELECT *, YEAR(`createdAt`) AS `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING creationYear > 2002;',
-          context: QueryGenerator,
-          needsSequelize: true
-        }, {
           title: 'HAVING clause works with where-like hash',
           arguments: ['myTable', function(sequelize) {
             return {

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

@@ -255,9 +255,6 @@ if (dialect.match(/^postgres/)) {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS \"count\" FROM \"foo\";'
         }, {
-          arguments: ['myTable', {where: ["foo='bar'"]}],
-          expectation: "SELECT * FROM \"myTable\" WHERE foo='bar';"
-        }, {
           arguments: ['myTable', {order: ['id']}],
           expectation: 'SELECT * FROM "myTable" ORDER BY "id";',
           context: QueryGenerator
@@ -385,18 +382,6 @@ if (dialect.match(/^postgres/)) {
           arguments: ['myTable', {group: ['name', 'title']}],
           expectation: 'SELECT * FROM \"myTable\" GROUP BY \"name\", \"title\";'
         }, {
-          title: 'HAVING clause works with string replacements',
-          arguments: ['myTable', function(sequelize) {
-            return {
-              attributes: ['*', [sequelize.fn('YEAR', sequelize.col('createdAt')), 'creationYear']],
-              group: ['creationYear', 'title'],
-              having: ['creationYear > ?', 2002]
-            };
-          }],
-          expectation: 'SELECT *, YEAR(\"createdAt\") AS \"creationYear\" FROM \"myTable\" GROUP BY \"creationYear\", \"title\" HAVING creationYear > 2002;',
-          context: QueryGenerator,
-          needsSequelize: true
-        }, {
           title: 'HAVING clause works with where-like hash',
           arguments: ['myTable', function(sequelize) {
             return {
@@ -461,10 +446,6 @@ if (dialect.match(/^postgres/)) {
           expectation: 'SELECT count(*) AS count FROM foo;',
           context: {options: {quoteIdentifiers: false}}
         }, {
-          arguments: ['myTable', {where: ["foo='bar'"]}],
-          expectation: "SELECT * FROM myTable WHERE foo='bar';",
-          context: {options: {quoteIdentifiers: false}}
-        }, {
           arguments: ['myTable', {order: ['id DESC']}],
           expectation: 'SELECT * FROM myTable ORDER BY id DESC;',
           context: {options: {quoteIdentifiers: false}}

test/unit/dialects/sqlite/query-generator.test.js

@@ -283,18 +283,6 @@ if (dialect === 'sqlite') {
           expectation: 'SELECT * FROM `myTable` GROUP BY name ORDER BY `id` DESC;',
           context: QueryGenerator
         }, {
-          title: 'HAVING clause works with string replacements',
-          arguments: ['myTable', function(sequelize) {
-            return {
-              attributes: ['*', [sequelize.fn('YEAR', sequelize.col('createdAt')), 'creationYear']],
-              group: ['creationYear', 'title'],
-              having: ['creationYear > ?', 2002]
-            };
-          }],
-          expectation: 'SELECT *, YEAR(`createdAt`) AS `creationYear` FROM `myTable` GROUP BY `creationYear`, `title` HAVING creationYear > 2002;',
-          context: QueryGenerator,
-          needsSequelize: true
-        }, {
           title: 'HAVING clause works with where-like hash',
           arguments: ['myTable', function(sequelize) {
             return {

test/unit/sql/select.test.js

@@ -4,6 +4,8 @@ const Support   = require(__dirname + '/../support'),
   DataTypes = require(__dirname + '/../../../lib/data-types'),
   Model = require(__dirname + '/../../../lib/model'),
   util = require('util'),
+  chai = require('chai'),
+  expect = chai.expect,
   expectsql = Support.expectsql,
   current   = Support.sequelize,
   sql       = current.dialect.QueryGenerator;
@@ -478,14 +480,58 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
   });
 
   suite('raw query', () => {
-    test('raw replacements', () => {
-      expectsql(sql.selectQuery('User', {
-        attributes: ['*'],
-        having: ['name IN (?)', [1, 'test', 3, 'derp']]
-      }), {
-        default: "SELECT * FROM [User] HAVING name IN (1,'test',3,'derp');",
-        mssql: "SELECT * FROM [User] HAVING name IN (1,N'test',3,N'derp');"
-      });
+    test('raw replacements for where', () => {
+      expect(() => {
+        sql.selectQuery('User', {
+          attributes: ['*'],
+          where: ['name IN (?)', [1, 'test', 3, 'derp']]
+        });
+      }).to.throw(Error, 'Support for literal replacements in the `where` object has been removed.');
+    });
+
+    test('raw replacements for nested where', () => {
+      expect(() => {
+        sql.selectQuery('User', {
+          attributes: ['*'],
+          where: [['name IN (?)', [1, 'test', 3, 'derp']]]
+        });
+      }).to.throw(Error, 'Support for literal replacements in the `where` object has been removed.');
+    });
+
+    test('raw replacements for having', () => {
+      expect(() => {
+        sql.selectQuery('User', {
+          attributes: ['*'],
+          having: ['name IN (?)', [1, 'test', 3, 'derp']]
+        });
+      }).to.throw(Error, 'Support for literal replacements in the `where` object has been removed.');
+    });
+
+    test('raw replacements for nested having', () => {
+      expect(() => {
+        sql.selectQuery('User', {
+          attributes: ['*'],
+          having: [['name IN (?)', [1, 'test', 3, 'derp']]]
+        });
+      }).to.throw(Error, 'Support for literal replacements in the `where` object has been removed.');
+    });
+
+    test('raw string from where', () => {
+      expect(() => {
+        sql.selectQuery('User', {
+          attributes: ['*'],
+          where: 'name = \'something\''
+        });
+      }).to.throw(Error, 'Support for `{where: \'raw query\'}` has been removed.');
+    });
+
+    test('raw string from having', () => {
+      expect(() => {
+        sql.selectQuery('User', {
+          attributes: ['*'],
+          having: 'name = \'something\''
+        });
+      }).to.throw(Error, 'Support for `{where: \'raw query\'}` has been removed.');
     });
   });
 });

test/unit/sql/where.test.js

@@ -2,10 +2,12 @@
 
 const Support   = require(__dirname + '/../support'),
   DataTypes = require(__dirname + '/../../../lib/data-types'),
-  util      = require('util'),
+  util = require('util'),
+  chai = require('chai'),
+  expect = chai.expect,
   expectsql = Support.expectsql,
-  current   = Support.sequelize,
-  sql       = current.dialect.QueryGenerator;
+  current = Support.sequelize,
+  sql = current.dialect.QueryGenerator;
 
 // Notice: [] will be replaced by dialect specific tick/quote character when there is not dialect specific expectation but only a default expectation
 
@@ -397,10 +399,12 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
     });
 
     suite('$raw', () => {
-      testsql('rank', {
-        $raw: 'AGHJZ'
-      }, {
-        default: '[rank] = AGHJZ'
+      test('should fail on $raw', () => {
+        expect(() => {
+          sql.whereItemQuery('rank', {
+            $raw: 'AGHJZ'
+          });
+        }).to.throw(Error, 'The `$raw` where property is no longer supported.  Use `sequelize.literal` instead.');
       });
     });