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

Mariadb migration can not create nullable timestamp column #14088

Open
2 of 7 tasks
arecaps opened this issue Feb 10, 2022 · 4 comments
Open
2 of 7 tasks

Mariadb migration can not create nullable timestamp column #14088

arecaps opened this issue Feb 10, 2022 · 4 comments
Labels
dialect: mariadb For issues and PRs. Things that involve MariaDB (and do not involve all dialects). type: bug

Comments

@arecaps
Copy link

arecaps commented Feb 10, 2022

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.
  • 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.
  • 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.
@ephys
Copy link
Member

ephys commented Feb 11, 2022

Could be fixed easily by always adding NULL is allowNull is true or undefined here

template += ' NOT NULL';

@ephys ephys added the dialect: mariadb For issues and PRs. Things that involve MariaDB (and do not involve all dialects). label Feb 11, 2022
@WikiRik
Copy link
Member

WikiRik commented Feb 11, 2022

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?

@arecaps
Copy link
Author

arecaps commented Feb 14, 2022

@ephys Can we just change that

template += ' NOT NULL';
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.

@ephys
Copy link
Member

ephys commented Feb 16, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: mariadb For issues and PRs. Things that involve MariaDB (and do not involve all dialects). type: bug
Projects
None yet
Development

No branches or pull requests

3 participants