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

Invalid ON CONFLICT written for upsert on models with conditional indexes (v6) #12595

Open
alexjeffburke opened this issue Aug 6, 2020 · 14 comments
Labels
type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@alexjeffburke
Copy link

alexjeffburke commented Aug 6, 2020

Issue Description

In version 6 query generation was made aware of the ON CONFLICT clause as is available in postgresql/sqlite. However, when upsert() is used in a model that has an index over two columns that is conditional on a third column, the conditional clause is not included in the ON CONFLICT and thus the upsert is rejected.

What are you doing?

The following is a minimal reproduction:

const { Sequelize, Model, DataTypes } = require('sequelize');
const sequelize = new Sequelize('sqlite::memory:'/*, { logging: false }*/);

class ConditionalIndexModel extends Model {}

ConditionalIndexModel.init({
  primary_id: {
    type: DataTypes.INTEGER,
    primaryKey: true,
    autoIncrement: true
  },
  first_id: {
    type: DataTypes.INTEGER,
    allowNull: false
  },
  second_id: {
    type: DataTypes.INTEGER,
    allowNull: false
  },
  stamped_at: DataTypes.DATE
}, {
  sequelize,
  modelName: 'conditional_index',

  indexes: [
    {
      unique: true,
      fields: ['first_id', 'second_id'],
      where: {
        stamped_at: null
      }   
    }
  ]
});

(async () => {
  await sequelize.sync();

  let instance;
  try {
    instance = await ConditionalIndexModel.upsert({
      first_id: 1,
      second_id: 2
    });
  } catch (e) {
    console.error(e);
    process.exit(1);
  }
  console.log(instance.toJSON());
})();

What do you expect to happen?

The upsert() to successfully complete.

What is actually happening?

DatabaseError [SequelizeDatabaseError]: SQLITE_ERROR: ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint

Additional context

As issued, the INSERT query looks as follows:

INSERT INTO `conditional_indices` (`first_id`,`second_id`,`createdAt`,`updatedAt`) VALUES (1,2,'2020-08-06 13:26:41.797 +00:00','2020-08-06 13:26:41.797 +00:00') ON CONFLICT (`first_id`,`second_id`) DO UPDATE SET `first_id`=EXCLUDED.`first_id`,`second_id`=EXCLUDED.`second_id`,`updatedAt`=EXCLUDED.`updatedAt`;

Based on some reading of the postgresql docs (), it appears a WHERE index_predicate is required between ON CONFLICT DO UPDATE to be able to disambiguate the conditional index. Thus in order for this query to succeed for this model, it must be written as follows:

INSERT INTO `conditional_indices` (`first_id`,`second_id`,`createdAt`,`updatedAt`) VALUES (1,2,'2020-08-06 13:26:41.797 +00:00','2020-08-06 13:26:41.797 +00:00') ON CONFLICT (`first_id`,`second_id`) WHERE `stamped_id` IS NULL DO UPDATE SET `first_id`=EXCLUDED.`first_id`,`second_id`=EXCLUDED.`second_id`,`updatedAt`=EXCLUDED.`updatedAt`;

When I looked in the code, the first unique index matching the columns being is chosen but then only the index keys are passed down into query generator insertQuery() function which writes them out as ON CONFLICT (...keys...), but the information is no longer available to it that there was a condition on the index so this is effectively ignored.

Environment

  • Sequelize version: 6.3.4
  • Node.js version: 13.11.0
  • Operating System: macOS

Issue Template Checklist

How does this problem relate to dialects?

I think this problem happens only for the following dialect(s): postresql, sqlite

Would you be willing to resolve this issue by submitting a Pull Request?

I'd be happy to work on a PR, but given the way this is currently handled an approach to addressing that the maintainers are comfortable with would likely need to be agreed.

@validkeys
Copy link

There is also a similar issue when using composite fields that require coalescing in the index.

If I try to create an index like so:

{
  indexes: [
    {
      name: 'doc_template_option_value_composite_idx',
      unique: true,
      fields: [
        [Sequelize.fn('COALESCE', Sequelize.col('organizationId')), '0'],
        [Sequelize.fn('COALESCE', Sequelize.col('teamId')), '0'],
        [Sequelize.fn('COALESCE', Sequelize.col('clientIdId')), '0'],
      ]
    }
  ]
}

The index is never used because the queryInterface is searching the "fields" array for a stringified column name instead of detecting that the field may be a sequelize method and parsing the column name from that.

const indexKey = indexKeys.find(fields => fields.includes(field));

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
@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 31, 2021
@papandreou
Copy link
Contributor

Still an issue for me :)

@github-actions github-actions bot removed the stale label Nov 1, 2021
@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 Nov 16, 2021
@papandreou
Copy link
Contributor

Still an issue.

@WikiRik WikiRik added type: feature For issues and PRs. For new features. Never breaking changes. and removed stale labels Nov 16, 2021
@mschoeffmann
Copy link

+1

1 similar comment
@andy-nhi
Copy link

+1

@dannomayernotabot
Copy link

still an issue

@utako
Copy link

utako commented Dec 2, 2021

+1

2 similar comments
@jourzero
Copy link

jourzero commented Dec 3, 2021

+1

@shreyansh-zazz
Copy link

+1

@shreyansh-zazz
Copy link

I tried the following and its working for me:

Model.upsert({
    property1: 1,
    property2: "sadasd",
    property3: 32323
}, {
   conflictFields: ['property1', 'property2']
})

@owengiri20
Copy link

I tried the following and its working for me:

Model.upsert({
    property1: 1,
    property2: "sadasd",
    property3: 32323
}, {
   conflictFields: ['property1', 'property2']
})

awesome thanks works for me

@alexnault
Copy link

For upsert to work with a partial index, I had to use conflictWhere (see: #13411) :

Model.upsert({
  conversationId: 'my-id',
  name: 'My conversation',
}, {
  conflictFields: ['conversationId'],
  conflictWhere: { deletedAt: null },
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests