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

How to update 'updatedAt' column in bulk update? #1962

Closed
alekbarszczewski opened this Issue Jun 25, 2014 · 7 comments

Comments

4 participants
@alekbarszczewski
Contributor

alekbarszczewski commented Jun 25, 2014

Using master version.

User.update({ field1: 'val1' },{ type: 'admin' }).done(...)
User.update({ field1: 'val1', updatedAt: new Date() },{ type: 'admin' }).done(...)

In both cases updatedAt column is ignored (it does not appear in SQL SET statement).

@mickhansen mickhansen added the Question label Jun 25, 2014

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jun 25, 2014

Strange that it is ignored in the second statement.
Not sure if this is because it's parsing the values through individual instances where they ignore read only attributes.

@seth-admittedly

This comment has been minimized.

Contributor

seth-admittedly commented Jun 26, 2014

I'm seeing the same problem with bulkCreate and the created_at attribute.

@janmeier

This comment has been minimized.

Member

janmeier commented Jun 29, 2014

@mickhansen , yep the validation and instance creation is the issue, read only attributes are skipped.

A simple fix is adding raw: true to https://github.com/sequelize/sequelize/blob/master/lib/model.js#L1375. But then setters will not be called? Is that expected behaviour for bulk?

edit did a quick check - all tests pass when adding raw: true

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jun 29, 2014

@janmeier i assume people would expect bulkCreate to work with setters and defaultValues etc so. We might not have tests for that though.

@janmeier

This comment has been minimized.

Member

janmeier commented Jul 1, 2014

Hmm yeah - so we should have an option that uses setters / getters, but also allows you to set read only attrs

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jul 1, 2014

Or we implement the updatedAt/createdAt logic ourselves in bulk create/update.

@janmeier

This comment has been minimized.

Member

janmeier commented Aug 10, 2014

@seth-admittedly Can't reproduce for bulkCreate on latest master - mind providing a test case?

@janmeier janmeier closed this in bb1d01c Aug 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment