sequelize: unique validation does not work (as described in documentation)

You’re welomce 😃

I agree that the documentation could be clearer on this. Sequelize is the only ORM I’ve ever used, so it is completely possible that you’re right here on what a Model should or should not do. But to me (and again I might be biased by lack of experience with other ORMs), your “automatic assumption” seems a bit extreme. Taking your own code, for example:

sequelize.define('User', {
  name: DataTypes.STRING,
  email: {
    type: DataTypes.STRING,
    validate: { isEmail: true },
    unique: true
  },
}, {});

Surely the type: DataTypes.STRING part is directly associated with the database itself, because it specifies the type of the column of the SQL table. That said, I don’t see why the unique: true couldn’t similarly be related to the schema definition itself (just my 2 cents).

But again I agree that a documentation improvement would be good. So don’t close the issue yet even if I convinced you.

By the way, if for some reason, in your specific situation, you can’t modify the database itself to add the unique constraint, you could create a workaround by defining a custom validator in your model that performs a getAll query and checks the results to guarantee uniqueness (if I’m not terribly mistaken, async validators are supported, so that should work).

Also, beyond the improvements in the documentation, this issue could become a ‘feature request’ for a different type of unique constraint, one that happens on the ORM layer by means of an extra query…

This issue has not been resolved.

The current way to fix this is by adding indices at the model and migration levels, and also to drop the database entirely and re-create it before running the migration – this was at least the case for me, undoing and applying the migrations was not correctly adding the constraint in the SQL table definition.

To clarify, here is what worked for me:

In the model definition:

// Options object passed in the Model.define() call
{
      indexes: [
        {
          unique: true,
          fields: ["email"] // Whatever other field you need to make unique
        }
      ]
}

In the migration file:

// Chained after queryInterface.createTable
.then(() => {
        queryInterface.addIndex("Users", ["email"], {
          unique: true
        });
});

I find it absurd that unique: true does not already apply this constraint to the table definition, unless one calls sync() which should never be used except for testing purposes, for reasons stated above.

to conclude for does coming here looking for a code solution here is a link to the docs with a code example findOrCreate

const [user, created] = await User.findOrCreate({
  where: { username: 'sdepold' },
  defaults: {
    job: 'Technical Lead JavaScript'
  }
});
console.log(user.username); // 'sdepold'
console.log(user.job); // This may or may not be 'Technical Lead JavaScript'
console.log(created); // The boolean indicating whether this instance was just created
if (created) {
  console.log(user.job); // This will certainly be 'Technical Lead JavaScript'
}

Thank you again. I have implemented a custom validator at the column level that asynchronously checks the database for existing email records with a different user ID, so my particular workaround exists. (I hope to generalize that to any abstract model though I need to learn my way around the internal of the instance variable first).

Your comments on “automatic assumption” are fair, so I thought of laying out as an argument that’s logical rather than (as in Rails) opinionated by tradition:

1. My principle assumption is that other than in prototyping, for any production grade system you always want to track the history of your schema changes for audit-ability.
2. Therefore if you ever “lose history” of migration, that’s a bad thing.
3. A model in most MVC systems represents the flattened latest state of schema.
4. The .sync() call from a model, therefore, syncs the schema to the latest state of the model, thus losing migration history.
5. So in a production-grade system, you never want to call .sync() from runtime code.
6. If you place a .sync() call in a migration script, your migration becomes dependent on your model. Thus when your model evolves, that historic migration changes what it does retroactively. Migrations shouldn’t depend on models.
7. Therefore, accepting 5 and 6, there’s never a non-experimental reason to call .sync()
8. Type definitions in a model allow for functionality extension, CRUD call validation, etc. (without these in fact they’d be boilerplate code). I’m not aware of anything “unique: true” enables other than syncing to the database.
9. Therefore, accepting 7 and 8, “unique: true” has no use in a non-experimental system.

I’d argue that if models had uniqueness validators (and I think evolution will select for this), the current “unique: true” concept will only add confusion without any big benefits (point 9), therefore we’re better off trading it in.

Thanks for reading my 2c 😃

On Thu, Jan 17, 2019 at 5:38 PM Pedro Augusto de Paula Barbosa < notifications@github.com> wrote:

You’re welomce 😃

I agree that the documentation could be clearer on this. Sequelize is the only ORM I’ve ever used, so it is completely possible that you’re right here on what a Model should or should not do. But to me (and again I might be biased by lack of experience with other ORMs), your “automatic assumption” seems a bit extreme. Taking your own code, for example:

sequelize.define(‘User’, { name: DataTypes.STRING, email: { type: DataTypes.STRING, validate: { isEmail: true }, unique: true }, }, {});

Surely the type: DataTypes.STRING part is directly associated with the database itself, because it specifies the type of the column of the SQL table. That said, I don’t see why the unique: true couldn’t similarly be related to the schema definition itself (just my 2 cents).

But again I agree that a documentation improvement would be good. So don’t close the issue yet even if I convinced you.

By the way, if for some reason, in your specific situation, you can’t modify the database itself to add the unique constraint, you could create a workaround by defining a custom validator in your model that performs a getAll query and checks the results to guarantee uniqueness (if I’m not terribly mistaken, async validators are supported, so that should work).

Also, beyond the improvements in the documentation, this issue could become a ‘feature request’ for a different type of unique constraint, one that happens on the ORM layer by means of an extra query…

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sequelize/sequelize/issues/10360#issuecomment-455396090, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBbLf6CgiDrHfj_fDlBOssJyQzk3h5Cks5vESWdgaJpZM4aG97k .

Hey everyone, I had the same issue and solved it by doing a migration like this:

await queryInterface.changeColumn('users', 'email', {
   type: Sequelize.STRING,
   allowNull: false,
   unique: true,
});

@papb I totally agree with @aminariana I really don’t understand the current behavior of the unique property why not implement a simple

IF NOT EXISTS

prefix to the create query in order to achieve this sure there is a performance consideration

but I think an ORM should prioritize ease of use and long term maintainability

if performance was my and most ORM users first consideration we would design our tables and write queries manually

also performing two queries to achieve this simple validation is a lot worst from a performance standpoint

and lastly this can also be achieved by indexing the email field but again indexing to perform validation to a field you are not planning to query by is just weird

I’m working with a PostgreSQL database. This is how my table is described now, after applying the changes mentioned in the previous comment:

                                       Table "public.Users"
  Column   |           Type           | Collation | Nullable |               Default
-----------+--------------------------+-----------+----------+-------------------------------------
 id        | integer                  |           | not null | nextval('"Users_id_seq"'::regclass)
 name      | character varying(255)   |           |          |
 username  | character varying(255)   |           |          |
 email     | character varying(255)   |           |          |
 password  | character varying(255)   |           |          |
 createdAt | timestamp with time zone |           | not null |
 updatedAt | timestamp with time zone |           | not null |
Indexes:
    "Users_pkey" PRIMARY KEY, btree (id)
    "users_email" UNIQUE, btree (email)

I remove those snippets from my model and migration files, i.e., go back to my initial problem state, drop the database and recreate it (db:drop followed by db:create followed by db:migrate), and this is how it is now described:

                                       Table "public.Users"
  Column   |           Type           | Collation | Nullable |               Default
-----------+--------------------------+-----------+----------+-------------------------------------
 id        | integer                  |           | not null | nextval('"Users_id_seq"'::regclass)
 name      | character varying(255)   |           |          |
 username  | character varying(255)   |           |          |
 email     | character varying(255)   |           |          |
 password  | character varying(255)   |           |          |
 createdAt | timestamp with time zone |           | not null |
 updatedAt | timestamp with time zone |           | not null |
Indexes:
    "Users_pkey" PRIMARY KEY, btree (id)

As can be seen, the email column is no longer unique.

Note: I ran db:migrate:undo:all and db:migrate to revert back to the correct behavior and it does work. So my earlier note on having to drop the database entirely was incorrect; it wasn’t working for me for whatever reason, previously, but I was likely doing something wrong.

In any case, the unique constraint in createTable is not applying the constraint. This is the relevant snippet of my createTable function:

email: {
          type: Sequelize.STRING,
          allowNull: { args: false, msg: "Please enter your email address" },
          validate: {
            isEmail: { args: true, msg: "Please enter a valid email address" }
          },
          unique: { args: true, msg: "Email already exists" }
},

The same problem, the unique constraint doesn’t work as expected on Model.init().

Update: When Sequelize is used to generate the tables, the unique constraint in the model definition works as expected using the technique in the Unique Constraint section of the docs.

But if you have pre-existing tables not generated by Sequelize, this does not appear to work by itself, even when using .sync().