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

Upsert uses a WHERE clause with both primary key and unique indices on Postgres #8552

Closed
DanielleB-R opened this issue Oct 27, 2017 · 9 comments

Comments

@DanielleB-R
Copy link

What are you doing?

I am using upsert to update models that have both a primary key and a composite unique constraint which is DEFERRABLE INITIALLY DEFERRED. When I try to change the members of the unique constraint I get a key uniqueness error.

Model:

module.exports = (sequelize, DataTypes) => {
  return sequelize.define(
    'rule',
    {
      id: {
        type: DataTypes.UUID,
        primaryKey: true,
        defaultValue: DataTypes.UUIDV4,
      },
      priority: {
        type: DataTypes.INTEGER,
        allowNull: false,
        defaultValue: 0,
        unique: 'rules_priority_parent_id'
      },
      parent_id: {
        type: DataTypes.UUID,
        allowNull: false,
        unique: 'rules_priority_parent_id'
      }
    },
    {
      tableName: 'rules',
      timestamps: true
    }
  )
}

Then do:

models.rule.create({
  id: 5,
  priority: 0,
  parent_id: 20
})

models.rule.create({
  id: 10,
  priority: 1,
  parent_id: 20
})

models.sequelize.transaction(() => (
  models.rule.upsert({
    id: 5,
    priority: 1,
    parent_id: 20
  })
    .then(() => models.rule.upsert({
      id: 10,
      priority: 0,
      parent_id: 20
    }))
))

What do you expect to happen?

The priority of the two rows to be swapped

What is actually happening?

Uniqueness constraint error

I get a constraint error on the uniqueness of the primary key.

Postgres output:

ERROR:  duplicate key value violates unique constraint "rules_pkey"
DETAIL:  Key (id)=(5) already exists.
CONTEXT:  SQL statement "UPDATE "rules" SET "priority"=1, "parent_id"=20, "id"=5, "updated_at"='2017-10-27 17:37:23.120 +00:00' WHERE ("id" = 5 OR ("priority" = 1 AND "parent_id" = 20))"
PL/pgSQL function pg_temp_3.sequelize_upsert() line 1 at SQL statement

I see a fix for sequelize@3 for the same problem (issue #6240, PR #6243), but it appears to still be a problem in sequelize@4.

Dialect: postgres
Dialect version: pg@6.4.2
Database version: 9.6.3
Sequelize version: 4.19.0
Tested with master branch: Yes

Note : Your issue may be ignored by maintainers if it's not tested against master branch OR does not follow issue template.

@sushantdhiman
Copy link
Contributor

Yea, definitely a bug. Please open a PR with changes from #6243 (Will need bit cleanup)

@iamjochem
Copy link
Contributor

is this not a case of the transaction being closed before the UPDATE statements are performed?

... it looks like there is a missing return, I'd be interested to know if the problem persists if you returned the first upsert call e.g.:

models.sequelize.transaction(() => (
  return models.rule.upsert({
    id: 5,
    priority: 1,
    parent_id: 20
  })
    .then(() => models.rule.upsert({
      id: 10,
      priority: 0,
      parent_id: 20
    }))
))

@sushantdhiman
Copy link
Contributor

@iamjochem No return is required as that arrow function is using implicit return format

@iamjochem
Copy link
Contributor

d'oh I mistook the parens for curlies ... sorry for the noise.

@osmanpontes
Copy link

It's still happening for me. Isn't fixed yet?
I'm not demanding, it's just to know.

@wl1711
Copy link

wl1711 commented May 21, 2020

still an issue for me too

@sushantdhiman
Copy link
Contributor

This is actually fixed by #12301, so only primary key will be selected as conflict target, when update data got PK value

const rule = sequelize.define(
  "rule",
  {
    id: {
      type: DataTypes.INTEGER,
      primaryKey: true,
    },
    priority: {
      type: DataTypes.INTEGER,
      allowNull: false,
      defaultValue: 0,
      unique: "rules_priority_parent_id",
    },
    parent_id: {
      type: DataTypes.INTEGER,
      allowNull: false,
      unique: "rules_priority_parent_id",
    },
  },
  {
    tableName: "rules",
    timestamps: true,
  }
);

(async () => {
  await sequelize.sync({ force: true });
  await rule.create({
    id: 5,
    priority: 0,
    parent_id: 20,
  });

  await rule.create({
    id: 10,
    priority: 1,
    parent_id: 20,
  });

  await sequelize.transaction((transaction) =>
    rule
      .upsert(
        {
          id: 5,
          priority: 1,
          parent_id: 20,
        },
        { transaction }
      )
      .then(() =>
        rule.upsert(
          {
            id: 10,
            priority: 0,
            parent_id: 20,
          },
          { transaction }
        )
      )
  );
})().catch((e) => console.error(e.original.message));
Executing (default): CREATE TABLE IF NOT EXISTS "rules" ("id" INTEGER , "priority" INTEGER NOT NULL DEFAULT 0, "parent_id" INTEGER NOT NULL, "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL, "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL, UNIQUE ("priority", "parent_id"), PRIMARY KEY ("id"));
Executing (default): SELECT i.relname AS name, ix.indisprimary AS primary, ix.indisunique AS unique, ix.indkey AS indkey, array_agg(a.attnum) as column_indexes, array_agg(a.attname) AS column_names, pg_get_indexdef(ix.indexrelid) AS definition FROM pg_class t, pg_class i, pg_index ix, pg_attribute a WHERE t.oid = ix.indrelid AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND t.relkind = 'r' and t.relname = 'rules' GROUP BY i.relname, ix.indexrelid, ix.indisprimary, ix.indisunique, ix.indkey ORDER BY i.relname;
Executing (default): INSERT INTO "rules" ("id","priority","parent_id","createdAt","updatedAt") VALUES ($1,$2,$3,$4,$5) RETURNING "id","priority","parent_id","createdAt","updatedAt";
Executing (default): INSERT INTO "rules" ("id","priority","parent_id","createdAt","updatedAt") VALUES ($1,$2,$3,$4,$5) RETURNING "id","priority","parent_id","createdAt","updatedAt";
Executing (4c0b09a0-b009-4410-8b65-adaf07e063f3): START TRANSACTION;
Executing (4c0b09a0-b009-4410-8b65-adaf07e063f3): INSERT INTO "rules" ("id","priority","parent_id","createdAt","updatedAt") VALUES ($1,$2,$3,$4,$5) ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id","priority"=EXCLUDED."priority","parent_id"=EXCLUDED."parent_id","updatedAt"=EXCLUDED."updatedAt" RETURNING "id","priority","parent_id","createdAt","updatedAt";
Executing (4c0b09a0-b009-4410-8b65-adaf07e063f3): ROLLBACK;

duplicate key value violates unique constraint "rules_priority_parent_id_key"

But it won't work for OP, as evident from SQL log for my SSCCE. When conflict target is using id it fails for rules_priority_parent_id_key. In simple terms, at one given moment a row will exists with different PK but same rules_priority_parent_id_key value.

I don't think OP's case can be covered unless constraints are deferred from transaction, which we support already.

Finally, I am closing this issue. As original request to only use PK as conflict target (when possible) has now been adopted by #12301

@wl1711
Copy link

wl1711 commented May 25, 2020

Would you mind to explain more on "it won't work for OP", I don't quite get why it won't work on my case #12292

As original request to only use PK as conflict target (when possible) has now been adopted by #12301

do you mean my case will be fixed by #12301?

@sushantdhiman
Copy link
Contributor

sushantdhiman commented May 26, 2020

Would you mind to explain more on "it won't work for OP", I don't quite get why it won't work on my case #12292

By OP, I meant original author of this thread. If you notice on first UPSERT (in my given SSCCE)

  1. We are trying to UPSERT id: 5, priority: 1, parent_id: 20
  2. Keep in mind id: 10, priority: 1, parent_id: 20 already exists.
  3. Choosing id as conflict target, internal INSERT will fail, as id: 5 already exists.
  4. So DB it will now try to update id: 5 record, with data priority: 1, parent_id: 20
  5. But that will fail as well, because of (2), rules_priority_parent_id constraint will block that update

do you mean my case will be fixed by #12301?

Actually no, I will reopen that thread and explain why It is fixed for all dialects, except MSSQL. Which we are tracking in #8634

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