feat(postgres): Add upsertIndex and upsertKeys options to BulkCreateOptions

[fnimick]

This allows the use of a unique index for a bulkCreate with
updateOnDuplicate set, in postgres. The unique index may be
a partial index.

Pull Request check-list

Please make sure to review and check all of these items:

• Does npm run test or npm run test-DIALECT pass with this change (including linting)?
• Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
• Have you added new tests to prevent regressions?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Did you update the typescript typings accordingly (if applicable)?
• Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

This change adds the ability to specify upsertIndex or upsertKeys in BulkCreateOptions, so that a ON CONFLICT clause can be generated using columns other than the table's primary key.

Note: I was not able to get the tests running on my machine, and would appreciate help writing a new test for this functionality.

Closes #11534, #11569

[codecov]

Codecov Report

Merging #11611 into master will increase coverage by 0.47%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #11611      +/-   ##
==========================================
+ Coverage   95.92%   96.40%   +0.47%     
==========================================
  Files          92       95       +3     
  Lines        8912     9128     +216     
==========================================
+ Hits         8549     8800     +251     
+ Misses        363      328      -35     

Impacted Files	Coverage Δ
lib/dialects/abstract/query-generator.js	97.40% <100.00%> (+<0.01%)	⬆️
lib/model.js	96.64% <100.00%> (+0.01%)	⬆️
lib/dialects/mariadb/index.js	100.00% <0.00%> (ø)
lib/dialects/mariadb/query.js	87.96% <0.00%> (ø)
lib/dialects/mariadb/connection-manager.js	100.00% <0.00%> (ø)
lib/sequelize.js	95.81% <0.00%> (+0.64%)	⬆️
lib/dialects/abstract/query-interface.js	93.76% <0.00%> (+1.86%)	⬆️
lib/dialects/mariadb/data-types.js	100.00% <0.00%> (+52.63%)	⬆️
lib/dialects/mariadb/query-generator.js	100.00% <0.00%> (+81.25%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f7fb5...42f64bf. Read the comment docs.

[fnimick]

Note: if anyone wants to use this while we go through the PR process, I have a patch-package patch available here: https://gist.github.com/fnimick/5dd3c6855cea6e000c86d1bc38876958

EDIT as of 6/9/2020: I have a new patch for sequelize 5.21.12 to support this in postgres and sqlite: https://gist.github.com/fnimick/313091df3ecc6c5e5f17be1e0950c33f

[papb]

Hello! I see you are a first-time contributor, thank you for taking the time to help Sequelize! I hope to see more PRs from you in the future!

To help you write tests, can you please first show usage examples of your feature?

[fnimick]

A usage example for upsertIndex:

model.bulkCreate(data, {
  updateOnDuplicate: ['updateField'],
  upsertIndex: 'unique_index',
});

NOTE: the index in question here must be defined on the model as well.

A usage example for upsertKeys with a partial index:

model.bulkCreate(data, {
  updateOnDuplicate: ['updateField'],
  upsertKeys: {
    fields: ['unique_field_one', 'unique_field_two'],
    where: { deleted_at: { [Op.is]: null } },
  },
});

A usage example for upsertKeys:

model.bulkCreate(data, {
  updateOnDuplicate: ['updateField'],
  upsertKeys: ['unique_field_one', 'unique_field_two'],
});

[fnimick]

@papb just updated this to resolve conflicts with #11666 . Still not sure how to write tests but can investigate if necessary.

[papb]

Hello 😬 thank you very much!!

[papb]

If you are having trouble to write tests, I suggest you try playing around with sequelize-sscce, if you can make short pieces of code that verify what you want, converting them to real tests will be almost trivial :) Let me know!

[molinajulian]

Hi everyone! 👋 Good job in this PR 🎉.
I recently have a problem with bulkInsert because of this bug
I think that it's a breaking change for version 5.x, it would be nice if we don't change the interface and we use the unique keys in the model for know if exist a composite unique keys or keep this change for versión 6.x and release a fix in another PR for 5.x.
@fnimick if you need help for tests, you can tell me

[fnimick]

Closed because a better approach has been merged in - see #11984

[fnimick]

@thaddeusmt went to implement my use case for this PR with your updated updateOnDuplicate and realized there's a case that my branch handles, but yours (I don't think) does - a partial unique index. In our case, I have a unique index on two fields only when a third field is null, and I generate an ON CONFLICT clause based on that index. Do you see an easier way to add support for that without needing to add the new functionality that I added here?

[fnimick]

@papb I've now added postgres specific tests for this. Please let me know if my style of writing them is OK.

[fnimick]

I've now also added sqlite tests, since this feature is supported by sqlite as well, and have improved the documentation on the new options.

[fnimick]

@papb is there anything I can do to move this forward? Thanks!

[malammar]

Would love to help out to get this done. It's critical for bulkCreate usage on Postgres.

[ramezhanna]

This PR is urgent to get bulk upsert work correctly on postgres

[CharlesEgan]

Agreed this is urgently needed. Happy to help resolving the conflicts so this can move forward.

[fnimick]

Unfortunately this PR was not reviewed before the rewrite in Typescript was started, so not only are there many conflicts, some of the files I have changed no longer exist.

I asked in #12429 about the best strategy, but got no clear answer.

I would recommend doing a raw sql query for now - or using sequelize 5.21.1 with my patch, if you absolutely must have the ORM interface which matches this PR. There's no guarantee that this interface will be merged in though, so I would recommend raw queries instead. @CharlesEgan

https://gist.github.com/fnimick/313091df3ecc6c5e5f17be1e0950c33f

[sdepold]

Hey there :)

First of all: Thanks a bunch for your contribution to Sequelize! Much appreciated!
Second: Unfortunately we haven't had the chance to look into this ever since you created it. Sorry for that!

A couple of months ago, we have switched from master to main branch as our primary development branch and hence this PR is now outdated :(

If you still think this change is making sense, please consider recreating the PR against main. Thanks in advance and sorry for the additional work.

✌️

[WikiRik]

Closing this since it is still against master. Can be reopened if that changes.