1054 add not null constraint causes data too long exception

[nhumblot]

Environment

Liquibase Version: 4.5.0

Liquibase Integration & Version: Spring Boot / Maven in my case but not specific to this integration

Liquibase Extension(s) & Version: N/A

Database Vendor & Version: MySQL

Operating System Type & Version: Not OS specific but I'm running Fedora 34

Pull Request Type

• Bug fix (non-breaking change which fixes an issue.)
• Enhancement/New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Fix #1054

A clear and concise description of the issue being addressed. Additional guidance here.

• Describe the actual problematic behavior.
• Ensure private information is redacted.

Steps To Reproduce

See:

• addNotNullConstraint causes "Data too long" Exception for BIT(1) fields #1054 (comment)
• addNotNullConstraint causes "Data too long" Exception for BIT(1) fields #1054 (comment) (comment)

Actual Behavior

Liquibase generates invalid SQL when setting a default value to a boolean column.

Taken from #1054 (comment)

<changeSet id="XXX" author="XXX">
    <addNotNullConstraint tableName="XX" columnName="XX" defaultNullValue="1" columnDataType="BIT(1)"/>
</changeSet>

Generates the following error:

Error setting up or running Liquibase: Migration failed for change set xxx.xml::xxxx::XXX:
[ERROR]      Reason: liquibase.exception.DatabaseException: (conn=28) Data too long for column

Expected/Desired Behavior

Liquibase generates a valid SQL update statement in the form of:

UPDATE FOO SET BAR = false WHERE BAR IS NULL

Fast Track PR Acceptance Checklist:

<

!--- If you're unsure about any of these, just ask us in a comment. We're here to help|width=200,height=183!

-->

• Build is successful and all new and existing tests pass
• Added Unit Test(s)
• Added Integration Test(s)
• Added Test Harness Test(s)
• Documentation Updated

Need Help?

Come chat with us on our discord channel

---

Dev Handoff Notes (Internal Use)

Links

• Fixed Issue: addNotNullConstraint causes "Data too long" Exception for BIT(1) fields #1054
• Test Results: https://github.com/liquibase/liquibase/pull/2071/checks

Testing

• Steps to Reproduce: addNotNullConstraint causes "Data too long" Exception for BIT(1) fields #1054
• Guidance:
  • Impact: only impacts <addNotNullConstraint> when dataType is a boolean like field for that database. But the logic is applied for all database types

Dev Verification

I used a changelog like this:

<changeSet id="1" author="XXX">
        <createTable tableName="test_table">
            <column name="id" type="int"/>
            <column name="col_name_bit" type="BIT(1)"/>
            <column name="col_name_boolean" type="BOOLEAN"/>
        </createTable>
    </changeSet>

    <changeSet id="2" author="XXX">
        <insert tableName="test_table">
            <column name="id" valueNumeric="1"/>
            <column name="col_name_bit" valueNumeric="0"/>
            <column name="col_name_boolean" valueBoolean="false"/>
        </insert>
        <insert tableName="test_table">
            <column name="id" valueNumeric="2"/>
        </insert>
    </changeSet>

    <changeSet id="test-bit" author="XXX">
        <addNotNullConstraint tableName="test_table" columnName="col_name_bit" defaultNullValue="1" columnDataType="BIT(1)"/>
    </changeSet>

    <changeSet id="test-bool" author="XXX">
        <addNotNullConstraint tableName="test_table" columnName="col_name_boolean" defaultNullValue="true" columnDataType="BOOLEAN"/>
    </changeSet>

With the fix, the update correctly runs.

Test Requirements (Liquibase Internal QA)

The manual testing is only required for MySQL database.

Setup

Execute update command against MySQL database (changelog file is attached):

liquibase update --changelog-file lb2187-changelog.xml --labels create,fill

Verify that table test_table was successfully created and has 2 rows with data as shown below

!Screenshot from 2021-12-15 18-43-12.png|width=25%!

Manual Tests

Verify that update-sql generates correct sql code for bit type.

• execute liquibase update-sql --changelog-file lb2187-changelog.xml --labels bit
• expected sql code:

UPDATE [db_name].test_table SET col_name_bit = 1 WHERE col_name_bit IS NULL;
ALTER TABLE [db_name].test_table MODIFY col_name_bit BIT(1) NOT NULL;

Verify that update is successful for bit type

• execute liquibase update --changelog-file lb2187-changelog.xml --labels bit
• update is successful
• no exception is thrown
• col_name_bit value for row with id=2 is now 1 instead of [NULL]

Verify that update-sql generates correct sql code for boolean type.

• execute liquibase update-sql --changelog-file lb2187-changelog.xml --labels bool
• expected sql code:

UPDATE [db_name].test_table SET col_name_boolean = true WHERE col_name_boolean IS NULL;
ALTER TABLE [db_name].test_table MODIFY col_name_boolean BIT(1) NOT NULL;

Verify that update is successful for boolean type

• execute liquibase update --changelog-file lb2187-changelog.xml --labels bool
• update is successful
• no exception is thrown
• col_name_boolean value for row with id=2 is now 1 instead of [NULL]

Automated Tests

• A new test-harness test is required to verify fix behavior against other supported database types.

┆Issue is synchronized with this Jira Bug by Unito

[nvoxland]

I pushed the current version of master to your fork to fix the build issue.

[molivasdat]

Hi @nhumblot Thanks for creating the fix.

A member of the Liquibase team will take a look at your contribution and may suggest:

• Changes based on guidelines for PR submission
• Additional tests (unit and/or integration) and/or Test Harness Tests
• Provide other feedback, like requesting changes.

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

[nvoxland]

I pushed a change to your fork that replaces the custom boolean parsing with existing functions that do something similar. That should make it a bit more generally applicable.

I was hoping to not keep the fix isolated to booleans/bits, but the current datatype APIs don't have the functionality I'd need yet. At some point we can improve it, but for not booleans/bits are likely the biggest source of this "data type too small" error