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

[CORE-3202] Fix NUMBER not compatible with H2 #3098

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

nick318
Copy link
Contributor

@nick318 nick318 commented Jul 21, 2022

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

Fixes NUMBER not compatible with H2 in PostreSql compatibility mode (or suggested for h2 in general).
Full description is here - https://liquibase.jira.com/browse/CORE-3202

As H2 supports numeric, we can use it as a default type as for others DBs

@kataggart kataggart added the DBH2 label Jul 22, 2022
@kataggart kataggart added this to To Do in Conditioning++ via automation Jul 22, 2022
@kataggart kataggart added the BKeyPlatform Key Platform label Jul 22, 2022
@nvoxland nvoxland added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Jul 27, 2022
@github-actions
Copy link

github-actions bot commented Jul 27, 2022

Unit Test Results

  4 620 files  ±  0    4 620 suites  ±0   34m 32s ⏱️ + 2m 32s
  4 597 tests +  3    4 378 ✔️  -   1     219 💤 +4  0 ±0 
54 336 runs  +36  49 312 ✔️ +32  5 024 💤 +4  0 ±0 

Results for commit 668ac01. ± Comparison against base commit a2dde51.

♻️ This comment has been updated with latest results.

@nvoxland nvoxland changed the title [CORE-3202] Fix NUMBER not compatible with H2 in PostreSql compatibility mode [CORE-3202] Fix NUMBER not compatible with H2 Aug 4, 2022
@nvoxland
Copy link
Contributor

nvoxland commented Aug 4, 2022

Code review and test results:

Things to be aware of:

  • Code makes sense and is isolated to h2
  • Test harness "diff" test is failing for what seems to be other reasons

Things to worry about:

  • Nothing

@nvoxland nvoxland added autocandidate SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Aug 4, 2022
@suryaaki2 suryaaki2 requested a review from nvoxland August 5, 2022 13:24
Copy link
Contributor

@XDelphiGrl XDelphiGrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • PR fixes a long-standing H2 bug where explicitly defined 'numeric' datatype in changesets was replaced with 'number', which is unsupported on H2.
    • An example of a changeset that was generating invalid SQL and now produces valid SQL containing NUMERIC is:
- changeSet:
      changes:
      - addColumn:
          tableName: talent
          columns:
          - column:
              name: rate
              type: numeric(20, 2)
  • New unit tests added for H2.

Copy link
Contributor

@KushnirykOleh KushnirykOleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated test-harness build failure. It is failing due to this branch being outdated, bringing latest master changes should fix the issue. Or this failure can be ignored as we already know the reason.
I'll take care of test-harness part after that

@nvoxland nvoxland merged commit 52fd51f into liquibase:master Aug 9, 2022
Conditioning++ automation moved this from To Do to Done Aug 9, 2022
@nick318 nick318 deleted the CORE-3202 branch August 10, 2022 07:36
@kataggart kataggart added this to the NEXT milestone Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autocandidate BKeyPlatform Key Platform DBH2 SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions sprint2022-31 TypeBug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants