sequelize icon indicating copy to clipboard operation
sequelize copied to clipboard

Mariadb migration can not create nullable timestamp column

Open arecaps opened this issue 3 years ago • 5 comments

Issue Creation Checklist

[x] I have read the contribution guidelines

Bug Description

Although for other datatypes, such a varchar, the default for MariaDb is Allow Null is true, but for timestamps it is false and it is handled differently, see the documentation and more docs here But is says there

This automatic initialization for NULL values can also be explicitly disabled for a column that uses the TIMESTAMP data type by specifying the NULL attribute for the column. In this case, if the column's value is set to NULL, then the column's value will actually be set to NULL.

One should therefore be able to specify when creating a timestamp column that it should be nullable, like this.

up: (queryInterface, Sequelize) => {
     queryInterface.addColumn("account", "created_at", {
       type: "TIMESTAMP",
       allowNull: true,
    }),
}

What do you expect to happen?

That should run the following query: (Note the NULL)

ALTER TABLE `account` ADD `created_at` TIMESTAMP NULL;

What is actually happening?

It is running this instead: (Note the NULL is missing)

ALTER TABLE `account` ADD `created_at` TIMESTAMP;

Additional context

This is a problem, because I want to add this field to an existing table, and if I don't make it nullable, it will insert the current timestamp into each existing row, (as mentioned in documentation linked above).
I can do it like this, but I believe it is still a bug.

    queryInterface.sequelize.query(
        "ALTER TABLE `account` ADD `created_at` TIMESTAMP NULL;",
      )

Environment

  • Sequelize version: 6.5.0
  • Node.js version: 16.13.2

Bug Report Checklist

How does this problem relate to dialects?

  • [ ] I think this problem happens regardless of the dialect.
  • [X] I think this problem happens only for the following dialect(s): Mariadb
  • [ ] I don't know, I was using PUT-YOUR-DIALECT-HERE, with connector library version XXX and database version XXX

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.
  • [x] 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.

arecaps avatar Feb 10 '22 16:02 arecaps

Could be fixed easily by always adding NULL is allowNull is true or undefined here https://github.com/sequelize/sequelize/blob/491ff159cda4aab72bad558fce533fc98e09ae7f/src/dialects/mysql/query-generator.js#L373

ephys avatar Feb 11 '22 12:02 ephys

Is this something you want to tackle in a PR @ephys or a guideline for where someone else can start with in tackling this issue?

WikiRik avatar Feb 11 '22 14:02 WikiRik

@ephys Can we just change that https://github.com/sequelize/sequelize/blob/491ff159cda4aab72bad558fce533fc98e09ae7f/src/dialects/mysql/query-generator.js#L373 to ?

if (attribute.allowNull === false) {
      template += ' NOT NULL';
    } else {
       template += ' NULL';

I can open a PR if that is all that needs to be done.

arecaps avatar Feb 14 '22 16:02 arecaps

It should be! But we should also add an integration test to ensure NULL can be inserted in a nullable TIMESTAMP table with mariadb

ephys avatar Feb 16 '22 09:02 ephys

For those looking for a quick fix, you can actually have your MariaDB server make up for Sequelize generating bad SQL by using the explicit_defaults_for_timestamp config variable (see documentation : https://mariadb.com/docs/server/ha-and-performance/optimization-and-tuning/system-variables/server-system-variables#explicit_defaults_for_timestamp).

By setting this to 1 in your MariaDB config MariaDB will always consider that timestamp columns should have explicit DEFAULT values and not use the implicit DEFAULT CURRENT_TIMESTAMP (as per https://mariadb.com/docs/server/reference/data-types/date-and-time-data-types/timestamp) that conflicts with a defaultValue : null in a model definition.

This config option is on by default in newer versions (>=10.10) so this Sequelize bug may not even be noticed in those environments.

Goz-0 avatar Jun 23 '25 15:06 Goz-0