fix(sqlite): retrieve primary key on upsert (#12991)

Co-authored-by: Pedro Augusto de Paula Barbosa <papb1996@gmail.com>

lib/dialects/sqlite/query.js

@@ -74,7 +74,7 @@ class Query extends AbstractQuery {
     let result = this.instance;
 
     // add the inserted row id to the instance
-    if (this.isInsertQuery(results, metaData)) {
+    if (this.isInsertQuery(results, metaData) || this.isUpsertQuery()) {
       this.handleInsertQuery(results, metaData);
       if (!this.instance) {
         // handle bulkCreate AI primary key
@@ -220,15 +220,7 @@ class Query extends AbstractQuery {
     const conn = this.connection;
     this.sql = sql;
     const method = this.getDatabaseMethod();
-    let complete;
-    if (method === 'exec') {
-      // exec does not support bind parameter
-      sql = AbstractQuery.formatBindParameters(sql, this.options.bind, this.options.dialect || 'sqlite', { skipUnescape: true })[0];
-      this.sql = sql;
-      complete = this._logQuery(sql, debug);
-    } else {
-      complete = this._logQuery(sql, debug, parameters);
-    }
+    const complete = this._logQuery(sql, debug, parameters);
 
     return new Promise((resolve, reject) => conn.serialize(async () => {
       const columnTypes = {};
@@ -250,13 +242,9 @@ class Query extends AbstractQuery {
           }
         }
 
-        if (method === 'exec') {
-          // exec does not support bind parameter
-          conn[method](sql, afterExecute);
-        } else {
-          if (!parameters) parameters = [];
-          conn[method](sql, parameters, afterExecute);
-        }
+        if (!parameters) parameters = [];
+        conn[method](sql, parameters, afterExecute);
+
         return null;
       };
 
@@ -437,10 +425,7 @@ class Query extends AbstractQuery {
   }
 
   getDatabaseMethod() {
-    if (this.isUpsertQuery()) {
-      return 'exec'; // Needed to run multiple queries in one
-    }
-    if (this.isInsertQuery() || this.isUpdateQuery() || this.isBulkUpdateQuery() || this.sql.toLowerCase().includes('CREATE TEMPORARY TABLE'.toLowerCase()) || this.options.type === QueryTypes.BULKDELETE) {
+    if (this.isInsertQuery() || this.isUpdateQuery() || this.isUpsertQuery() || this.isBulkUpdateQuery() || this.sql.toLowerCase().includes('CREATE TEMPORARY TABLE'.toLowerCase()) || this.options.type === QueryTypes.BULKDELETE) {
       return 'run';
     }
     return 'all';

test/integration/model/upsert.test.js

@@ -491,7 +491,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       }
 
       if (current.dialect.supports.returnValues) {
-        describe('with returning option', () => {
+        describe('returns values', () => {
           it('works with upsert on id', async function() {
             const [user0, created0] = await this.User.upsert({ id: 42, username: 'john' }, { returning: true });
             expect(user0.get('id')).to.equal(42);
@@ -576,26 +576,26 @@ describe(Support.getTestDialectTeaser('Model'), () => {
               expect(created).to.be.false;
             }
           });
-        });
 
-        it('should return default value set by the database (upsert)', async function() {      
-          const User = this.sequelize.define('User', {
-            name: { type: DataTypes.STRING, primaryKey: true },
-            code: { type: Sequelize.INTEGER, defaultValue: Sequelize.literal(2020) }
+          it('should return default value set by the database (upsert)', async function() {
+            const User = this.sequelize.define('User', {
+              name: { type: DataTypes.STRING, primaryKey: true },
+              code: { type: Sequelize.INTEGER, defaultValue: Sequelize.literal(2020) }
+            });
+
+            await User.sync({ force: true });
+
+            const [user, created] = await User.upsert({ name: 'Test default value' }, { returning: true });
+
+            expect(user.name).to.be.equal('Test default value');
+            expect(user.code).to.be.equal(2020);
+
+            if (dialect === 'sqlite' || dialect === 'postgres') {
+              expect(created).to.be.null;
+            } else {
+              expect(created).to.be.true;
+            }
           });
-    
-          await User.sync({ force: true });
-    
-          const [user, created] = await User.upsert({ name: 'Test default value' }, { returning: true });
-      
-          expect(user.name).to.be.equal('Test default value');
-          expect(user.code).to.be.equal(2020);
-
-          if (dialect === 'sqlite' || dialect === 'postgres') {
-            expect(created).to.be.null;
-          } else {
-            expect(created).to.be.true;
-          }
         });
       }
     });

test/integration/sequelize/query.test.js

@@ -605,23 +605,5 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {
           .to.eventually.deep.equal([{ 'sum': '5050' }]);
       });
     }
-
-    if (Support.getTestDialect() === 'sqlite') {
-      it('binds array parameters for upsert are replaced. $$ unescapes only once', async function() {
-        let logSql;
-        await this.sequelize.query('select $1 as foo, $2 as bar, \'$$$$\' as baz', { type: this.sequelize.QueryTypes.UPSERT, bind: [1, 2], logging(s) { logSql = s; } });
-        // sqlite.exec does not return a result
-        expect(logSql).to.not.include('$one');
-        expect(logSql).to.include('\'$$\'');
-      });
-
-      it('binds named parameters for upsert are replaced. $$ unescapes only once', async function() {
-        let logSql;
-        await this.sequelize.query('select $one as foo, $two as bar, \'$$$$\' as baz', { type: this.sequelize.QueryTypes.UPSERT, bind: { one: 1, two: 2 }, logging(s) { logSql = s; } });
-        // sqlite.exec does not return a result
-        expect(logSql).to.not.include('$one');
-        expect(logSql).to.include('\'$$\'');
-      });
-    }
   });
 });