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

bulkCreate with unique partial index in postgres #12774

Closed
2 of 7 tasks
ramezhanna opened this issue Oct 20, 2020 · 15 comments · Fixed by #13420
Closed
2 of 7 tasks

bulkCreate with unique partial index in postgres #12774

ramezhanna opened this issue Oct 20, 2020 · 15 comments · Fixed by #13420
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). P4: nice to have For issues that are not bugs. status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@ramezhanna
Copy link

ramezhanna commented Oct 20, 2020

Issue Description

model bulkCreate function with updateOnDuplicate don't support partial unique index for postgres:
[SequelizeDatabaseError]: there is no unique or exclusion constraint matching the ON CONFLICT specification

What are you doing?

While trying to use bulkCreate function with unique partial index in postgres i could manage to use the model and create the query to bulkCreate

postgres partial unique index

create unique index table_partial_idx
    on table (key1, key2, key3)
    where (key3 not in (3,4));

Model Defination

const Table = sequelize.define('Table', {
    id: {
      allowNull: false,
      type: DataTypes.STRING,
      primaryKey: true,
      defaultValue: DataTypes.UUIDV4,
    },
    key1: {
      type: DataTypes.STRING,
      unique: 'table_partial_idx',
    },
    key2: {
      type: DataTypes.STRING,
      unique: 'table_partial_idx',
    },
    key3: {
      type: DataTypes.INTEGER,
      unique: 'table_partial_idx',
    },
    key4: {
      type: DataTypes.DATE,
      defaultValue: DataTypes.NOW,
    },
    key5: {
      type: DataTypes.STRING,
    },
  }, {
    indexes: [{
      name: 'table_partial_idx',
      unique: true,
      fields: ['key1', 'key2', 'key3'],
      where: {
        status: {
          [Op.notin]: [3,4],
        },
      },
    }],
  });

Model usage

await sequelize[entityName].bulkCreate([...data] {
        updateOnDuplicate: ['key4', 'key5'],
        returning: true,
      }

What do you expect to happen?

Expected to generate a insert query with On Conflict key and a where condition
Expected query

insert
	into
	"alarms" ("id", "key1", "key2", "key3", "key4", "key5")
values (uuid_generate_v4(), 'text', 'text', 1, '2020-10-20 12:01:32.143 +00:00', 'test')
 ON CONFLICT (key1, key2, key3)  where (key3 not in (3,4))
DO UPDATE SET key4 = EXCLUDED.key4 , key5 = EXCLUDED.key5;

What is actually happening?

generate a insert query with On Conflict key but without a where condition that leaded to

DatabaseError [SequelizeDatabaseError]: there is no unique or exclusion constraint matching the ON CONFLICT specification

output query

insert
	into
	"alarms" ("id", "key1", "key2", "key3", "key4", "key5")
values (uuid_generate_v4(), 'text', 'text', 1, '2020-10-20 12:01:32.143 +00:00', 'test')
 ON CONFLICT (key1, key2, key3) 
DO UPDATE SET key4 = EXCLUDED.key4 , key5 = EXCLUDED.key5;

Additional context

Add any other context or screenshots about the feature request here.

Environment

  • Sequelize version: 6.3.5
  • Node.js version: v12.19.0
  • Operating System: Ubuntu 20.04

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.
  • I think this problem happens only for the following dialect(s):
  • I don't know, I was using postgres, with connector library version pg:8.2.1 & pg-hstore:pg-hstore and database version 11

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.
@almgwary
Copy link

any updates on this ?

@Kaiwen-Song
Copy link

upsert also has the same issue

@rraina
Copy link

rraina commented Jan 20, 2021

Having same issue with upsert which can break people migrating from v5.

@papb @sushantdhiman given this was a important feature for v6, is this something we could prioritize fixing?

@schmod
Copy link
Contributor

schmod commented Feb 17, 2021

I've been looking at the ramifications of v6's upsert changes for our upgrade, and I think this probably needs to be called out in the upgrade guide, as it's a big change that does not necessarily have a clean upgrade path.

My $0.02 is that we probably need to add a new parameter to upsert and bulkCreate that allows Postgres users to manually tell Sequelize what to put in the query's ON CONFLICT clause. There are many scenarios where it's going to be impossible for the ORM to determine which index to use.

Even now, the current logic that automatically guesses which index to use is fairly naive. The current query generator chooses the first unique index that includes a column present in the values passed into the upsert operation.

However, this might be a bad assumption if a table has multiple overlapping constraints!

For example, if a table has uniqueness constraints on (A,B) and (A,C), if I call model.upsert({ A: 1, B: 2, C: 3 }), Sequelize is probably going to generate a query that includes ON CONFLICT (A,B) DO UPDATE. However, if I've actually violated the constraint on A,C, Postgres will throw an error.

In the above scenario, I don't know if there's any way for Sequelize to choose the correct index to use. The best real-world solution would be to create a new uniqueness constraint on (A,B,C) (the union of all other uniqueness constraints). However, again, Sequelize currently only picks the first constraint that it finds – not the best one.


Partial indices make this much, much more complicated, and I honestly can't think of a good/elegant way to support them, apart from adding a parameter that allows users to manually specify the index name, ie. model.upsert(values, { onConflictIndex: 'the_partial_idx_on_my_table' });

@alexandruluca
Copy link

Any updates on this? It would be great to allow users what constraint to use

wbourne0 added a commit to wbourne0/sequelize that referenced this issue May 11, 2021
Add support for where clauses used in the ON CONFLICT part of the queries generated by model.bulkCreate and model.upsert for usage >

Closes sequelize#13240, sequelize#13066, sequelize#13031, sequelize#12774, sequelize#12742, sequelize#12595, sequelize#11656
@wbourne0 wbourne0 added dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). P4: nice to have For issues that are not bugs. status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. type: feature For issues and PRs. For new features. Never breaking changes. labels Aug 5, 2021
@wbourne0
Copy link
Member

wbourne0 commented Aug 5, 2021

Worth noting that sqlite can also use this.

I've got PRs for both Model.bulkCreate (#13420) and Model.upsert (#13411).

I also created an issue specifically for Model.upsert at #13412.

@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the stale label Oct 29, 2021
@schmod
Copy link
Contributor

schmod commented Oct 29, 2021

Not stale.

@github-actions github-actions bot removed the stale label Oct 30, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2021

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

@github-actions github-actions bot added the stale label Nov 7, 2021
@schmod
Copy link
Contributor

schmod commented Nov 7, 2021

This is still an issue

@github-actions github-actions bot removed the stale label Nov 8, 2021
@adanzweig
Copy link

still an issue

@david-cryptohub
Copy link

Still an issue in sequelize@6.20.1

@bennyh960
Copy link

Still an issue

1 similar comment
@denizd1
Copy link

denizd1 commented Oct 22, 2022

Still an issue

@kr99
Copy link

kr99 commented Nov 16, 2022

Sequelize is an awesome library! We love using it.

As a general user-experience and developer-experience guideline, smart auto-configurations can never get it right 100% of the time--you will NEVER get it right 100%. This is why MS-Word autocorrect lets you override autocorrect settings, and most IDEs let you suppress eslint & other warnings by line/file/project. This is why test libraries have features for coverage exceptions.

This may be a great feauture, but we need a fallback/override.

For now, my team will look at hiding uniqueness constraints from the Sequelize models (leaving them only in the db)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). P4: nice to have For issues that are not bugs. status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.