Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bulkCreate with updateOnDuplicate updates the record (updatedAt) even if it is not changed #17247

Open
3 of 6 tasks
akshay-vishnoi opened this issue Apr 4, 2024 · 3 comments
Labels
type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@akshay-vishnoi
Copy link

Issue Creation Checklist

  • I understand that my issue will be automatically closed if I don't fill in the requested information
  • I have read the contribution guidelines

Bug Description

I am using bulkCreate to create/update the records. If the records are already existing then they should get updated only if the required field has changed and also should automatically update the updatedAt timestamp.

The issue I am facing is I have to provide the updatedAt field in updateOnDuplicate then only it updates the timestamp, and if I do so, then it updates the timestamp even if the required column is not changed, if I don't provide the updatedAt field in the updateOnDuplicate options then it only updates the required field and doesn't update the timestamp.

Reproducible Example

Let's say that my table structure is:

Table Foo (a:int unique, b: string, createdAt: datetime, updatedAt: datetime)
and it has two records

a b createdAt updatedAt
1 'one' 2024-04-04 01:07:25 2024-04-04 01:07:25
2 'two' 2024-04-04 01:07:25 2024-04-04 01:07:25

if I use bulkCreate operation on table foo:

model.bulkcreate([{a:1, b:'three'},{a:2, b: 'two'}], { updateOnDuplicate: ['b', 'updatedAt']})

then it updates updatedAt for both records even when there was no change in second record (with b = 'two').

And if I use bulkCreate operation like the following

model.bulkcreate([{a:1, b:'three'},{a:2, b: 'two'}], { updateOnDuplicate: ['b']})

then it updates value of b for first record but doesn't update the timestamp updatedAt for the first record.

What do you expect to happen?

Ideally, the updatedAt timestamp should be handled by sequelize package, and it should update it only if there is any change in field b.

What is actually happening?

Environment

  • Sequelize version: 6.32.1
  • Node.js version: 18.12.1
  • If TypeScript related: TypeScript version: 5.1.3
  • Database & Version: Mysql 8.3.0
  • Connector library & Version: mysql2 3.6.0

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I will need guidance.
  • No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • No, I don't have the time, and I understand that I will need to wait until someone from the community or maintainers is interested in resolving my issue.

Indicate your interest in the resolution of this issue by adding the 👍 reaction. Comments such as "+1" will be removed.

@akshay-vishnoi akshay-vishnoi added pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: bug labels Apr 4, 2024
@ephys
Copy link
Member

ephys commented Apr 4, 2024

In MySQL, this query is done using INSERT ... ON DUPLICATE KEY UPDATE. I believe we could have a big IF statement for the updated_at column that would look like this:

-- assumes "a" is unique
INSERT INTO daily_events (a, b, updated_at)
  VALUES (1, 2, '2024-04-04 04:04:04') AS new
ON DUPLICATE KEY UPDATE
  b = new.b,
  updated_at = IF(a <> new.a, new.updated_at, updated_at);

Similarly, updated_at should automatically be added to updateOnDuplicate if updateOnDuplicate is specified and silent is not false

@ephys ephys added type: feature For issues and PRs. For new features. Never breaking changes. and removed type: bug pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet labels Apr 4, 2024
@akshay-vishnoi
Copy link
Author

akshay-vishnoi commented Apr 4, 2024

Interesting, how can we do it with Sequelize? Does Sequelize support it?

Looks like Sequelize we can only provide the array of the fields, please check here

updateOnDuplicate?: Array<keyof TAttributes>;

@ephys
Copy link
Member

ephys commented Apr 5, 2024

Nothing clean as far as I know. Maybe you'll have some change with Model.upsert, but I doubt it, hence why I turned this into a feature request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests

2 participants