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

diffChangeLog on fields with a comment cause 'default' and 'null' to flip flop #2607

Closed
Tedward747 opened this issue Mar 6, 2022 · 3 comments

Comments

@Tedward747
Copy link

Environment

Liquibase Version: 4.8.0
Liquibase Integration & Version: CLI
Liquibase Extension(s) & Version: N/A
Database Vendor & Version: MySQL 8.0.17
Operating System Type & Version: Windows 10

Description

Multiple runs of liquibase --changeLogFile=changes/<name>.mysql.sql diffChangeLog followed by liquibase update flips 'NOT NULL' and 'DEFAULT 0' back and forth on fields that have a comment. This may be related to #2563 but I'm not positive since my issue is about multiple runs and not calling setColumnRemarks.

First diff returns 3 relevant lines. The first line sets the fields up properly:

CREATE TABLE mainNotifications ( ... , notifyBy ENUM('0', '1', '2') DEFAULT '0' NOT NULL COMMENT '0=all, 1=email, 2=mobile (for confirming new email address or mobiles)', overrideConfirmation ENUM('0', '1') DEFAULT '0' NOT NULL COMMENT 'Used when confirming a new email or mobile', ... );

However the next two lines both drop DEFAULT '0' NOT NULL:

ALTER TABLE mainNotifications MODIFY COLUMN notifyBy ENUM('0', '1', '2') COMMENT '0=all, 1=email, 2=mobile (for confirming new email address or mobiles)';
ALTER TABLE mainNotifications MODIFY COLUMN overrideConfirmation ENUM('0', '1') COMMENT 'Used when confirming a new email or mobile';

Running update then diffChangeLog again adds DEFAULT '0' NOT NULL back over two lines for each field:

ALTER TABLE mainNotifications MODIFY notifyBy ENUM('0', '1', '2') NOT NULL;
ALTER TABLE mainNotifications ALTER notifyBy SET DEFAULT '0';
ALTER TABLE mainNotifications MODIFY overrideConfirmation ENUM('0', '1') NOT NULL;
ALTER TABLE mainNotifications ALTER overrideConfirmation SET DEFAULT '0';

Third run drops them again:

ALTER TABLE mainNotifications MODIFY COLUMN notifyBy ENUM('0', '1', '2') COMMENT '0=all, 1=email, 2=mobile (for confirming new email address or mobiles)';
ALTER TABLE mainNotifications MODIFY COLUMN overrideConfirmation ENUM('0', '1') COMMENT 'Used when confirming a new email or mobile';

etc.

Manually deleting the 2nd and 3rd lines of the first diff does seem to permanantly fix the problem, but that's only really helpful if you know what to do before you spend a few hours bashing your head against a wall trying to figure out what's going on. :)

Steps To Reproduce

Set up database with a table that has fields with DEFAULT '0', NOT NULL, and a comment
Set up second empty database

run liquibase --changeLogFile=changes/<name>.mysql.sql diffChangeLog followed by liquibase update a few times

Actual Behavior

Fields flip flop between DEFAULT and NULL being set and unset

Expected Behaviour

Set it the first time and don't change it unless there's actually a change

@kataggart
Copy link
Contributor

Thanks @Tedward747 for all the detail here. We will take a look and get back to you.

@kataggart kataggart added this to To Do in Conditioning++ via automation Mar 7, 2022
@ecarneiro01 ecarneiro01 self-assigned this Jun 16, 2022
@ecarneiro01
Copy link

@nvoxland I tried to reproduce this in 4.12, it looks like this is still an issue.

The only difference is that the first generated changelog doesn't drop the NOT NULL and DEFAULT properties. The generated SQL is the following:

-- liquibase formatted sql

-- changeset enzoc:1655746251694-1
CREATE TABLE mainNotifications (id INT NOT NULL, notifyBy ENUM('2', '1', '0') DEFAULT '0' NOT NULL COMMENT '0=all, 1=email, 2=mobile (for confirming new email address or mobiles)', overrideConfirmation ENUM('1', '0') DEFAULT '0' NOT NULL COMMENT 'Used when confirming a new email or mobile');

However, if you run liquibase update in the target DB and then you do diffChangeLog again it is creating an SQL changelog with changesets even though both DBs should be equal at this point. If you repeat this last step you will see the behavior where applying the generated changelog sets and unsets the DEFAULT and NOT NULL properties.

@ecarneiro01 ecarneiro01 moved this from To Do to In Development in Conditioning++ Jun 20, 2022
@ecarneiro01 ecarneiro01 removed their assignment Jun 20, 2022
@nvoxland
Copy link
Contributor

nvoxland commented Jul 6, 2022

This is an issue with how mysql handles column modification. Liquibase does not have a general "redefine everything about this column" change type, but mysql requires that. So any changes we do generate like "add a comment" or "add a default value" will risk losing the other settings on there, like you see happening.

It's an example of how the generated changesets are only a starting point, and you should make sure to inspect them and modify them to be correct. For mysql/mariadb that will also include handling these "modify column" changesets as needed.

I did create #3045 to better warn when there are changes being ran that may be an issue.

@nvoxland nvoxland closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2022
Conditioning++ automation moved this from In Development to Done Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants