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

Bulk updates without fetching/Related joins #402

Closed
gregbty opened this issue Jun 26, 2014 · 3 comments
Closed

Bulk updates without fetching/Related joins #402

gregbty opened this issue Jun 26, 2014 · 3 comments

Comments

@gregbty
Copy link

gregbty commented Jun 26, 2014

It's been a pleasure using this library. It influenced me to promisify my entire code base which is much cleaner than using the async lib and node callbacks.

I just have two questions:

I noticed that relations are fetched one at a time:
db.User()
 .fetchAll({withRelated: ['roles']})
 .then(function (user) {
    res.json(user);
  });

The following would produce two queries:

select * from `users`;

select * from `user_roles` where `user`.`id` in (?, ?)

instead of:

select * from `users` left join `user_roles` on `users`.`id` = `user_roles`.`userid`

Is there a reason for this, or was this something that was planned in a future release?


Is it possible to do actual bulk updates:
db.User
 .where('lastLoginDate', '<', moment().toDate().subtract('d', '60'))
 .save({enabled: false});

// mysql: update `users` set `enabled` = ? where `lastLoginDate` < '?' 

instead of:

db.User
 .where('lastLoginDate', '<', moment().toDate().subtract('d', '60'))
 .fetchAll()
 .then(function (users) {
  return users.invokeThen('save', {enabled: false});
});

/** mysql:

select * from `users` where `lastLoginDate` < '?' 

//repeated for x amount of users
update `users` set `enabled` = ? where `id` = ?

**/
@tgriesser
Copy link
Member

Is there a reason for this, or was this something that was planned in a future release?

This is intentional, as the join requires that you know all of the columns in advance, which bookshelf does not require, you'd need a good way of aliasing all of the columns such that there are no collisions, and then you'd need to iterate over the entire result set to deal with parsing/deduping the columns, which in javascript might actually be slower than dealing with the extra initial query. This is also how ActiveRecord generally deals with eager loading.

By making it two separate queries, you also gain the ability to easily constrain the subqueries at different levels, a la:

db.User()
 .fetchAll({withRelated: [{
    'roles': function(qb) {
       qb.where('status', active);
    }}, 'roles.info']
  }).then(function (user) {
    res.json(user);
  });

which would provide:

select * from `users`;

select * from `users_roles` where `users_roles`.`user_id` in (?, ?) and `status` = 'active'

select * from `info` where `role_id` in (?, ?, ?...)

There will however be improvements in coming releases, and the ability to do better joins for constraints will be provided (and possibly a simple way of doing something like you mentioned).

Is it possible to do actual bulk updates:

Not with Bookshelf, you'd do it with knex... the .query() method called with no arguments gives you a knex builder instance based on the current tableName...

User.query()
       .where('lastLoginDate', '<', moment().toDate().subtract('d', '60'))
       .update({enabled: false}).then(...

@AntJanus
Copy link

@tgriesser would your update solution update the timestamps as well? So that if I did:

User
  .query()
  .where('id', 25)
  .update({ enabled: false }).then(...

The updated_at column would yield a new timestamp?

@rhys-vdw
Copy link
Contributor

@AntJanus updated_at will not be modified in the code you've shown.

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

No branches or pull requests

5 participants