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

Cannot add nullable TIMESTAMP column #63

Closed
CYBAI opened this issue Sep 24, 2020 · 20 comments
Closed

Cannot add nullable TIMESTAMP column #63

CYBAI opened this issue Sep 24, 2020 · 20 comments

Comments

@CYBAI
Copy link

CYBAI commented Sep 24, 2020

Problem

With adding a new nullable TIMESTAMP column in schema, mysqldef will generate a DDL without specifying NULL or not and result in an error like following

ERROR 1067 (42000): Invalid default value for 'created_at'
Environment
  • mysqldef: 0.5.14
  • mysql: 5.7

Test Case

Initial Schema
CREATE TABLE user (
  id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
  name VARCHAR(128) DEFAULT 'konsumer'
) Engine=InnoDB DEFAULT CHARSET=utf8mb4;
New Schema
CREATE TABLE user (
  id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
  name VARCHAR(128) DEFAULT 'konsumer',
  created_at TIMESTAMP NULL DEFAULT NULL
) Engine=InnoDB DEFAULT CHARSET=utf8mb4;

Current Generated DDLs

ALTER TABLE user ADD COLUMN created_at timestamp DEFAULT null AFTER name

Expected Generated DDLs

ALTER TABLE user ADD COLUMN created_at timestamp NULL DEFAULT null AFTER name

I tried to run the expected one locally and it works fine on MySQL.

@k0kubun
Copy link
Collaborator

k0kubun commented Sep 25, 2020

Thank you for your report. While I can fix sqldef's behavior to print the "Expected Generated DDLs", I couldn't reproduce your issue:

$ mysql -uroot -e 'create database test;'

$ cat /tmp/a.sql
CREATE TABLE user (
  id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
  name VARCHAR(128) DEFAULT 'konsumer'
) Engine=InnoDB DEFAULT CHARSET=utf8mb4;

$ go run cmd/mysqldef/mysqldef.go -uroot test < /tmp/a.sql
-- Apply --
CREATE TABLE user (
  id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
  name VARCHAR(128) DEFAULT 'konsumer'
) Engine=InnoDB DEFAULT CHARSET=utf8mb4;

$ cat /tmp/b.sql
CREATE TABLE user (
  id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
  name VARCHAR(128) DEFAULT 'konsumer',
  created_at DATETIME NULL DEFAULT NULL
) Engine=InnoDB DEFAULT CHARSET=utf8mb4;

$ go run cmd/mysqldef/mysqldef.go -uroot test < /tmp/b.sql
-- Apply --
ALTER TABLE user ADD COLUMN created_at datetime DEFAULT null AFTER name;

It feels like the column gets NOT NULL if NULL is not explicitly specified, which I believe isn't the default behavior of MySQL.

https://dev.mysql.com/doc/refman/5.7/en/create-table.html
If neither NULL nor NOT NULL is specified, the column is treated as though NULL had been specified.

Is it configured explicitly in your environment as such? I'd like to know how to do that for running tests with the mode.

@CYBAI
Copy link
Author

CYBAI commented Sep 28, 2020

@k0kubun I tried to check my environment and didn't see any special settings though, I think you're right that the default behavior should be NULL. I will try to investigate more about my environment and see if I can provide how to test in this mode. Thanks!

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 4, 2020

Let me close this issue until the investigation is done. Please feel free to reopen this issue when you finish it or you need the workaround first.

@k0kubun k0kubun closed this as completed Oct 4, 2020
@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

@k0kubun I think I might need your help for the workaround first 🙏 Thank you!

I tried to ask my colleague and confirmed following configurations in our docker-compose.yml 👇

    command:
    - --character-set-server=utf8mb4
    - --collation-server=utf8mb4_bin
    - --general-log-file=/var/log/mysql/query.log
    - --general-log=true
    - --long-query-time=1
    - --slow-query-log-file=/var/log/mysql/slow.log
    - --slow-query-log=true
    environment:
    - LANG=C.UTF-8
    - MYSQL_DATABASE=${MYSQL_DATABASE}

I can't find any of them can have impact to the NULL / NON NULL configuration :/

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

I think I might need your help for the workaround first

OK.

I can't find any of them can have impact to the NULL / NON NULL configuration :/

but you could reproduce the issue using the docker-compose.yml? If possible, I would appreciate an entire docker-compose.yml which could reproduce the behavior (at least I need to know the Docker image name if public).

@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

I think I might need your help for the workaround first

OK.

Thank you!

I can't find any of them can have impact to the NULL / NON NULL configuration :/

but you could reproduce the issue using the docker-compose.yml? If possible, I would appreciate an entire docker-compose.yml which could reproduce the behavior (at least I need to know the Docker image name if public).

Unfortunately, the docker-compose.yml is in our private repo >_<

Btw, I'm using mysql:5.7 docker image. The most interesting (or I should say weird) thing is that I can run those DDLs in #63 (comment) with accessing into the mysql container and creating a new test database. :/

@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

oh, wait! because I'm running commands with Makefile in our codebase, I just checked the Makefile and see we set 'SET GLOBAL FOREIGN_KEY_CHECKS=0' before running mysqldef.

Edit: With removing the FOREIGN_KEY_CHECKS=0, I still get the invalid value error 😭

@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

@k0kubun gotcha! I finally found we set sql_mode in our Haskell backend service instead of from the docker-compose.yaml :/ which it sets NO_ZERO_DATE and NO_ZERO_IN_DATE.

(sorry for that I can't share the exact piece of code here because it's in our private repo 🙏)

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

Great, thanks! I'll try reproducing and fixing it.

@k0kubun k0kubun reopened this Oct 6, 2020
@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

Great, thanks! I'll try reproducing and fixing it.

Thank you so much!

Sorry for that I just report that too quick >_< I found I still can reproduce the error with removing the 2 configs. Let me confirm them properly and report it again! Thank you!

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

Oh yeah, neither NO_ZERO_DATE nor NO_ZERO_IN_DATE didn't reproduce it. I tried some other strict modes but it didn't do it either. I tried to look for such other options, but I've failed to find it so far.

Let me confirm them properly and report it again! Thank you!

Thanks, let me (close this issue for now and) wait for the report first 🙏

@k0kubun k0kubun closed this as completed Oct 6, 2020
@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

Let me confirm them properly and report it again! Thank you!

Thanks, let me (close this issue for now and) wait for the report first 🙏

Thank you! I wish I can report the root cause soon!

@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

With doing some more investigation, I found I still got the invalid default value error even removing all sql_mode by SET GLOBAL sql_mode = ''. (With searching the error on the internet, most solutions say just set sql_mode to empty)

However, I found I can fix the issue with turning on explicit_defaults_for_timestamp.

But, I'm very confused 🤔 With reading the documentation of explicit_defaults_for_timestamp, it should be default in OFF (that my db is also default in OFF) so I'd expect I can reproduce the invalid default issue in any new created database but it didn't :/

I feel I don't actually find the root cause though... 😞 Though I'm able to fix the issue with setting --explicit_defaults_for_timestamp=ON on my side, could you please help me to see if the configuration is OFF in your environment? Thank you!

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

could you please help me to see if the configuration is OFF in your environment?

Sure, it was:

mysql> show variables like '%explicit%';
+---------------------------------+-------+
| Variable_name                   | Value |
+---------------------------------+-------+
| explicit_defaults_for_timestamp | OFF   |
+---------------------------------+-------+
1 row in set (0.00 sec)

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

Just an additional info:

https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_explicit_defaults_for_timestamp
TIMESTAMP columns not explicitly declared with the NULL attribute are automatically declared with the NOT NULL attribute.

After seeing that, I tested TIMESTAMP instead of DATETIME:

 CREATE TABLE user (
   id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
   name VARCHAR(128) DEFAULT 'konsumer',
+  created_at TIMESTAMP NULL DEFAULT NULL
 ) Engine=InnoDB DEFAULT CHARSET=utf8mb4;

and got an error similar to yours:

$ go run cmd/mysqldef/mysqldef.go -uroot test < /tmp/a.sql
-- Apply --
ALTER TABLE user ADD COLUMN created_at timestamp DEFAULT null AFTER name;
2020/10/05 23:28:27 Error 1067: Invalid default value for 'created_at'
exit status 1

and this doesn't happen with DATETIME, although I'm not sure the doc intends to also cover DATETIME by "TIMESTAMP columns" or not.

@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

At this point, I could write a failing test with the default configuration. I can implement the original proposal, because it's not really a workaround but a reasonable fix.

@k0kubun k0kubun reopened this Oct 6, 2020
@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

After seeing that, I tested TIMESTAMP instead of DATETIME:

Aaaaah!! Thanks for catching it!!!! I mis-interpret DATETIME as TIMESTAMP in my head 😭

@CYBAI CYBAI changed the title Cannot add nullable datetime column Cannot add nullable TIMESTAMP column Oct 6, 2020
@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

I just changed the issue name and description to match the exact issue!

@k0kubun k0kubun closed this as completed in 50197f9 Oct 6, 2020
@k0kubun
Copy link
Collaborator

k0kubun commented Oct 6, 2020

Thanks to all your helps, I fixed the issue at 50197f9 and started the release as v0.7.2.

@CYBAI
Copy link
Author

CYBAI commented Oct 6, 2020

Thank your so much as well for your patience and quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants