fix: upsert should set createdAt by default while it not set (#277)

Co-authored-by: JimmyDaddy <[email protected]>

src/drivers/abstract/spellbook.js

@@ -356,15 +356,14 @@ function formatInsert(spell) {
       }
     } else {
       for (const name in involved) {
-        // upsert should not update createdAt
-        if (updateOnDuplicate && createdAt && name === createdAt) continue;
         attributes.push(Model.attributes[name]);
       }
     }
 
     for (const entry of attributes) {
       columns.push(entry.columnName);
-      if (updateOnDuplicate && createdAt && entry.name === createdAt) continue;
+      if (updateOnDuplicate && createdAt && entry.name === createdAt 
+        && !(Array.isArray(updateOnDuplicate) && updateOnDuplicate.includes(createdAt))) continue;
       updateOnDuplicateColumns.push(entry.columnName);
     }
 
@@ -385,7 +384,6 @@ function formatInsert(spell) {
     }
     for (const name in sets) {
       const value = sets[name];
-      // upsert should not update createdAt
       columns.push(Model.unalias(name));
       if (value instanceof Raw) {
         values.push(SqlString.raw(value.value));

src/drivers/mysql/spellbook.js

@@ -49,7 +49,10 @@ module.exports = {
     const sets = [];
     // Make sure the correct LAST_INSERT_ID is returned.
     // - https://stackoverflow.com/questions/778534/mysql-on-duplicate-key-last-insert-id
-    sets.push(`${escapeId(primaryColumn)} = LAST_INSERT_ID(${escapeId(primaryColumn)})`);
+    // if insert attributes include primary column, `primaryKey = LAST_INSERT_ID(primaryKey)` is not need any more
+    if (!columns.includes(primaryColumn)) {
+      sets.push(`${escapeId(primaryColumn)} = LAST_INSERT_ID(${escapeId(primaryColumn)})`);
+    }
     sets.push(...columns.map(column => `${escapeId(column)}=VALUES(${escapeId(column)})`));
 
     return `ON DUPLICATE KEY UPDATE ${sets.join(', ')}`;

test/integration/postgres.test.js

@@ -2,6 +2,7 @@
 
 const assert = require('assert').strict;
 const path = require('path');
+const sinon = require('sinon');
 
 const { connect, raw } = require('../..');
 
@@ -57,6 +58,28 @@ describe('=> upsert', function () {
       new Post({ id: 1, title: 'New Post', createdAt: raw('CURRENT_TIMESTAMP()'), updatedAt: raw('CURRENT_TIMESTAMP()') }).upsert().toString(),
       `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"`
     );
+    const date = new Date(2017, 11, 12);
+    const fakeDate = date.getTime();
+    sinon.useFakeTimers(fakeDate);
+    assert.equal(
+      new Post({ id: 1, title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(),
+      `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"`
+    );
+    assert.equal(
+      new Post({ title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(),
+      `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"`
+    );
+    // default set createdAt
+    assert.equal(
+      new Post({ id: 1, title: 'New Post' }).upsert().toString(),
+      `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"`
+    );
+
+    assert.equal(
+      Post.upsert({ title: 'New Post' }).toSqlString(),
+      `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"`
+    );
+
   });
 
   it('upsert returning multiple columns', function() {

test/integration/sqlite.test.js

@@ -2,6 +2,8 @@
 
 const assert = require('assert').strict;
 const path = require('path');
+const sinon = require('sinon');
+
 const { connect, raw, Bone } = require('../..');
 const { checkDefinitions } = require('./helpers');
 
@@ -63,5 +65,26 @@ describe('=> upsert (sqlite)', function () {
       new Post({ id: 1, title: 'New Post', createdAt: raw('CURRENT_TIMESTAMP()'), updatedAt: raw('CURRENT_TIMESTAMP()') }).upsert().toString(),
       `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"`
     );
+    const date = new Date(2017, 11, 12);
+    const fakeDate = date.getTime();
+    sinon.useFakeTimers(fakeDate);
+    assert.equal(
+      new Post({ id: 1, title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(),
+      `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"`
+    );
+    assert.equal(
+      new Post({ title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(),
+      `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"`
+    );
+    // default set createdAt
+    assert.equal(
+      new Post({ id: 1, title: 'New Post' }).upsert().toString(),
+      `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"`
+    );
+
+    assert.equal(
+      Post.upsert({ title: 'New Post' }).toSqlString(),
+      `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"`
+    );
   });
 });

test/integration/suite/basics.test.js

@@ -853,6 +853,16 @@ describe('=> Basic', () => {
           assert.equal(await post.upsert(), 0);
         });
 
+        it('bone.upsert should set createdAt default', async () => {
+          await (new Post({ title: 'New Post', isPrivate: 0 }).upsert());
+          const post = await Post.findOne('title = ?', 'New Post');
+          assert(post.createdAt);
+          const date = new Date(2017, 11, 12);
+          await (new Post({ title: 'post 1', isPrivate: 0, createdAt: date }).upsert());
+          const post1 = await Post.findOne('title = ?', 'post 1');
+          assert.deepEqual(post1.createdAt, date);
+        });
+
         it('bone.upsert() should return affectedRows', async function() {
           const post = new Post({ title: 'New Post', isPrivate: 0 });
           // INSERT ... UPDATE returns 1 if the INSERT branch were chosen
@@ -892,6 +902,16 @@ describe('=> Basic', () => {
           assert.equal(count, 1);
         });
 
+        it('Bone.upsert should set createdAt default', async () => {
+          await Post.upsert({ title: 'New Post', isPrivate: 0 });
+          const post = await Post.findOne('title = ?', 'New Post');
+          assert(post.createdAt);
+          const date = new Date(2017, 11, 12);
+          await Post.upsert({ title: 'post 1', isPrivate: 0, createdAt: date });
+          const post1 = await Post.findOne('title = ?', 'post 1');
+          assert.deepEqual(post1.createdAt, date);
+        });
+
         it('Bone.upsert() should return affectedRows', async function() {
           const tag = await Tag.create({ name: 'Sekiro', type: 1 });
           // INSERT ... UPDATE returns 1 if the INSERT branch were chosen
@@ -1156,6 +1176,57 @@ describe('=> Basic', () => {
       assert.equal(p2.title, 'Leah1');
     });
 
+    it('Bone.bulkCreate() updateOnDuplicate with createdAt default or set', async () => {
+      await User.bulkCreate([
+        { nickname: 'TYRAEL', email: '[email protected]', status: 1 },
+        { nickname: 'LEAH', email: '[email protected]', status: 1 },
+      ], {
+        updateOnDuplicate: true,
+      });
+
+      assert.equal(await User.count(), 2);
+      let p1 = await User.findOne({ email: '[email protected]' });
+      assert.equal(p1.nickname, 'TYRAEL');
+      const p1CreatedAt = p1.createdAt;
+      assert(p1CreatedAt);
+      let p2 = await User.findOne({ email: '[email protected]' });
+      assert.equal(p2.nickname, 'LEAH');
+      const p2CreatedAt = p2.createdAt;
+      assert(p2CreatedAt);
+
+      await User.bulkCreate([
+        { nickname: 'TYRAEL1', email: '[email protected]', status: 1 },
+        { nickname: 'LEAH1', email: '[email protected]', status: 1 },
+      ], {
+        updateOnDuplicate: [ 'nickname', 'status' ]
+      });
+
+      let p1Updated1 = await User.findOne({ email: '[email protected]' });
+      assert.equal(p1Updated1.nickname, 'TYRAEL1');
+      const p1Updated1CreatedAt = p1Updated1.createdAt;
+      assert.deepEqual(p1Updated1CreatedAt, p1CreatedAt);
+      let p2Updated1 = await User.findOne({ email: '[email protected]' });
+      assert.equal(p2Updated1.nickname, 'LEAH1');
+      const p2Updated1CreatedAt = p2Updated1.createdAt;
+      assert.deepEqual(p2Updated1CreatedAt, p2CreatedAt);
+
+      await User.bulkCreate([
+        { nickname: 'TYRAEL2', email: '[email protected]', status: 1 },
+        { nickname: 'LEAH2', email: '[email protected]', status: 1 },
+      ], {
+        updateOnDuplicate: [ 'nickname', 'status', 'createdAt' ]
+      });
+
+      let p1Updated2 = await User.findOne({ email: '[email protected]' });
+      assert.equal(p1Updated2.nickname, 'TYRAEL2');
+      const p1Updated2CreatedAt = p1Updated2.createdAt;
+      assert.notDeepEqual(p1Updated2CreatedAt, p1CreatedAt);
+      let p2Updated2 = await User.findOne({ email: '[email protected]' });
+      assert.equal(p2Updated2.nickname, 'LEAH2');
+      const p2Updated2CreatedAt = p2Updated2.createdAt;
+      assert.notDeepEqual(p2Updated2CreatedAt, p2CreatedAt);
+    });
+
     it('Bone.bulkCreate() should work with updateOnDuplicate keys', async () => {
       await User.bulkCreate([
         { nickname: 'Tyrael', email: '[email protected]', status: 1 },

test/unit/spell.test.js

@@ -55,9 +55,35 @@ describe('=> Spell', function() {
 
   it('insert ... on duplicate key update', function() {
     const date = new Date(2017, 11, 12);
+    const fakeDate = date.getTime();
+    sinon.useFakeTimers(fakeDate);
     assert.equal(
       new Post({ id: 1, title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(),
-      "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
+      "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
+    );
+    assert.equal(
+      new Post({ title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(),
+      "INSERT INTO `articles` (`is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
+    );
+    // default set createdAt
+    assert.equal(
+      new Post({ id: 1, title: 'New Post' }).upsert().toString(),
+      "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
+    );
+
+    assert.equal(
+      Post.upsert({ title: 'New Post' }).toSqlString(),
+      "INSERT INTO `articles` (`is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
+    );
+
+    assert.equal(
+      new Book({ name: 'Dark', price: 100 }).upsert().toSqlString(),
+      "INSERT INTO `books` (`name`, `price`, `gmt_create`, `gmt_modified`) VALUES ('Dark', 100, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `isbn` = LAST_INSERT_ID(`isbn`), `name`=VALUES(`name`), `price`=VALUES(`price`), `gmt_modified`=VALUES(`gmt_modified`)"
+    );
+
+    assert.equal(
+      new Book({ isbn: 10000, name: 'Dark', price: 100 }).upsert().toSqlString(),
+      "INSERT INTO `books` (`isbn`, `name`, `price`, `gmt_create`, `gmt_modified`) VALUES (10000, 'Dark', 100, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `isbn`=VALUES(`isbn`), `name`=VALUES(`name`), `price`=VALUES(`price`), `gmt_modified`=VALUES(`gmt_modified`)"
     );
   });
 
@@ -729,7 +755,7 @@ describe('=> Spell', function() {
   it('upsert with raw sql', function () {
     assert.equal(
       new Post({ id: 1, title: 'New Post', createdAt: raw('CURRENT_TIMESTAMP()'), updatedAt: raw('CURRENT_TIMESTAMP()') }).upsert().toString(),
-      "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
+      "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON DUPLICATE KEY UPDATE `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)"
     );
   });