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 ignoreDuplicates throws an error when empty response (due to ignored duplicates) #14470

Open
2 of 5 tasks
JacobLey opened this issue Apr 29, 2022 · 4 comments
Open
2 of 5 tasks

Comments

@JacobLey
Copy link
Contributor

JacobLey commented Apr 29, 2022

Issue Creation Checklist

Bug Description

SSCCE

#14469

See failing tests

What do you expect to happen?

When ignoreDuplicates=true is applied to a create operation, Sequelize should not reject with an EmptyResultError as that is a perfectly acceptable result.

What is actually happening?

Sequelize rejects with EmptyResultError.

For postgres, that is thrown here: https://github.com/sequelize/sequelize/blob/main/src/dialects/postgres/query.js#L262
Note lack of check against ignoreDuplicates flag

Additional context

The ignoreDuplicates is declared on model types, and the actual SQL generated appears to respect the option as documented.

The issue is when it actually hits duplicates and returns nothing, Sequelize does not expect that case and throws an error.

Environment

Sequelize version: 16.19.0
Node version: 16.15.0 (does not appear to be node-specific)
Database: Postgres 13 (should be impacted by any database that supports ignoreDuplicates)

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 don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.

Already have PR open for failing test, so would add onto that.

Two possible approaches I can think of:

  1. Simply check for ignoreDuplicates flag before throwing the empty error
  2. Convert an Insert query with ignoreDuplicates=true to an upsert internally (that updates nothing)

Former is easiest to implement, but also feels like there more be edge cases uncaught... Latter is probably a more robust solution but not sure if there are other side effects that will be triggered as a result

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has been open for 14 days without activity. It will be closed if no further activity occurs within the next 14 days. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label May 14, 2022
@JacobLey
Copy link
Contributor Author

Still an issue

@github-actions github-actions bot removed the stale label May 15, 2022
@hiendaovinh
Copy link

Temporary workaround: Using bulkCreate.

@pocketarc
Copy link

I'm running up against this issue now as well, in Sequelize 7. I might spend some time putting together a PR to resolve this, if it would be welcome.

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

4 participants