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

Create multi queries DB twice for every object #109

Closed
robbyphillips opened this issue Aug 17, 2020 · 3 comments
Closed

Create multi queries DB twice for every object #109

robbyphillips opened this issue Aug 17, 2020 · 3 comments

Comments

@robbyphillips
Copy link
Contributor

robbyphillips commented Aug 17, 2020

Currently, for every object in the data array, this adapter makes a separate create request and get request (unless using the $noSelect operator).

It looks like this is so feathers-objection can return the whole object, including any values that might be generated by the database, like id.

I'm opening this issue to see if we can allow some configuration to change this behavior, since this seems like a lot of overhead for bulk operations.

In particular, if we are using application generated ids for our entities (uuid, etc), we can always accomplish this same behavior with 1 batch insert, and 1 find using those ids.

Since we already know the id field of our entity via the service configuration, I wonder if it's enough to check that for our data array?

  • If all entities in the array have that field set, do the batch insert and single find query.
  • If not, default to the current method

If we don't want to do automatic detection like that, we could enable it with a configuration option.

Additional optimizations could also be implemented if we had a "postgres mode" using the returning() operator https://vincit.github.io/objection.js/api/query-builder/find-methods.html#returning.

Edit: it looks like feathers-knex does use returning() here: https://github.com/feathersjs-ecosystem/feathers-knex/blob/master/lib/index.js#L228

@dekelev
Copy link
Member

dekelev commented Aug 26, 2020

Thanks @robbyphillips, It's a possible enhancement, but should be tested with various databases.

Some databases doesn't return the generated ID and IN query is not always more performant than running each query separately.

@dekelev
Copy link
Member

dekelev commented Dec 12, 2020

@robbyphillips, PR #127 seems to close this issue. can you please verify?

@robbyphillips
Copy link
Contributor Author

Yes, it looks like it does. Thanks!

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

2 participants