Improve int/tinyint/smallint/bigint handling in H2

[MalloD12]

Impact

• Bug fix (non-breaking change which fixes expected existing functionality)
• Enhancement/New feature (adds functionality without impacting existing logic)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Implement support to consider integer precision by parsing to the specific integer type (tiny, integer or bigint) depending on the specified size.

Fixes #3250
Fixes #3300

Things to be aware of

• This change only impacts integer data type for H2.

Things to worry about

• Nothing.

Additional Context

• Nothing.

[github-actions]

Unit Test Results

  4 716 files  +  48    4 716 suites  +48   34m 15s ⏱️ -13s
  4 652 tests +  43    4 423 ✔️ +  43     229 💤 ±0  0 ❌ ±0 
55 044 runs  +528  49 976 ✔️ +528  5 068 💤 ±0  0 ❌ ±0 

Results for commit 04bc61c. ± Comparison against base commit 59ca536.

♻️ This comment has been updated with latest results.

[nvoxland]

I think it's fine to be checking the size here for int and converting to the correct actual types, since new h2 does not support precision like that.

But, the issue still exists where someone specifying smallint(3) or bigint(15) or whatever will still fail. So I think we need fixes to TinyIntType, SmallIntType, etc. that simply ignore the parameters passed since h2 doesn't support that. I don't think we need to do logic like "smallint(10) means bigint".

Also, could you add a changeset to the h2 integration test changelog with the different int, smallint, etc. types for columns to ensure the final SQL is valid?

[MalloD12]

I think it's fine to be checking the size here for int and converting to the correct actual types, since new h2 does not support precision like that.

But, the issue still exists where someone specifying smallint(3) or bigint(15) or whatever will still fail. So I think we need fixes to TinyIntType, SmallIntType, etc. that simply ignore the parameters passed since h2 doesn't support that. I don't think we need to do logic like "smallint(10) means bigint".

Also, could you add a changeset to the h2 integration test changelog with the different int, smallint, etc. types for columns to ensure the final SQL is valid?

@nvoxland If I didn't get it wrong, what you are asking for here is being addressed as part of this PR. Can you please have a look and confirm?

Thanks,
Daniel.

[nvoxland]

@MalloD12 yes, that other PR picks up the other problems, I hadn't seen that one yet. Since it's touching similar code we could have done it as a single PR addressing both issues, but fine to keep them separate since you've made both already.

Both could use integration test items for them still, though.

[MalloD12]

@MalloD12 yes, that other PR picks up the other problems, I hadn't seen that one yet. Since it's touching similar code we could have done it as a single PR addressing both issues, but fine to keep them separate since you've made both already.

Both could use integration test items for them still, though.

if you are ok with that, I can still unify these two PRs in just a single one and then close one of them? I think some of them already have integration test created, but yeah some others don't.

[nvoxland]

Code review and test results:

Things to be aware of:

• H2 doesn't support arguments on any of the int datatypes, so this correctly removes them if they are set
• Includes tests to ensure they are as expected
• Only impacts h2

Things to worry about:

• Nothing

[VeluchamySC]

Could you please include the fix for BOOLEAN Types as well?

Currently with H2 2.1.214, Liquibase is not able to translate 1 to TRUE (or) 0 to FALSE. With previous version of H2 1.4.200, Liquibase was able to translate it.

Steps to reproduce:
databaseChangeLog:

• changeSet:
  id: '1'
  author: author
  comment: 'initial schema'
  changes:
  • addColumn:
    tableName: order
    columns:
    • column:
      name: order_status
      type: VARCHAR(255)
      constraints:
      nullable: false
      value: NONE
  • update:
    columns:
    • column:
      name: order_status
      value: EXECUTED
      tableName: order
      where: executed = 1

Reason: liquibase.exception.DatabaseException: Values of types "BOOLEAN" and "INTEGER" are not comparable; SQL statement:
UPDATE PUBLIC.order SET order_status = 'EXECUTED' WHERE executed = 1 [90110-214] [Failed SQL: (90110) UPDATE PUBLIC.order SET order_status = 'EXECUTED' WHERE executed = 1]

[nvoxland]

@VeluchamySC that won't be something we can fix. The where clause can be arbitrarily complex and so it's something we can try to parse and understand. All we do is pass it along as the update statement's where clause exactly as you have it.

It looks like a change in what h2 will do for data type conversions between 1.x and 2.x. If you have just switched to h2 2.x you can update the statement to use executed = true. (You will need to add a validCheckSum tag to it knows you know what you are doing). If you still need to support both versions, you can use a changelog parameter like ${trueValue} which you set to 1 or true depending on the version of h2 you are using.

[XDelphiGrl]

H2 does not support precision in the SQL syntax, but it is valid to include precision in changeset datatype column definitions. This fix correctly "translates" integer datatypes specified with precision when SQL is generated for an H2 database, generating SQL with TINYINT, SMALLINT, INTEGER or BIGINT. Liquibase determines the correct type of integer following the H2 documentation here:

H2 Data Types

• Comprehensive set of integration tests added.
• No additional testing required.

APPROVED

[TomBenjamins]

Any progress on this issue? When can we expect this MR in a liquibase version?

greetings,
Tom

[MalloD12]

Hi @TomBenjamins We are looking for getting another review on this PR. Once that happens we will be ready to merge this. We are expecting to have this close soon to include it in the next release, which hopefully should be in about two weeks.

Thanks,
Daniel.-

[kamil-j]

Hi,
Has this issue been fixed for raw sql changes as well? If not, is there a plan to somehow address it? After migration from H2 1.4.200 to 2.1.214 scripts are failing with the same message.

Example changeset:
<sql> CREATE TABLE SampleTable ( id bigint(20) NOT NULL PRIMARY KEY AUTO_INCREMENT ) </sql>

If raw sqls won't be addressed then could you please suggest what to do with those old changelogs (already executed on envs like prod, but still used on test)?

[foxpluto]

Bump for the fix...