Fix BLOBs in loadData for MySQL/MariaDB #2595

MichaelCkr · 2022-03-03T11:39:48Z

Environment

Liquibase Version: 4.6.2, 4.8.0

Liquibase Integration & Version: maven

Liquibase Extension(s) & Version: -

Database Vendor & Version: MariaDB 10.3.27

Operating System Type & Version: linux

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)

closes #2519

Description

When we use loadData and there exists a BLOB column (preparedStatement necessary), then no data are populated in the DB at all. Even no exception is thrown, but in databasechangelog table the changeset is set as run.

Steps To Reproduce

see #2519

Actual Behavior

changeset with loadData is marked as successful in databasechangelog table
no data are populated in database
no exception or warning can be found in log

Expected/Desired Behavior

changeset with loadData is marked as successful in databasechangelog table
data are populated in database

Screenshots (if appropriate)

Additional Context

see #2519 and #2215

I'm not sure, if the intention of #2215, increasing the performance of MySql data loading, is still fulfilled with this change. The probably slow batch update is not used, but prepared statements are used.

Fast Track PR Acceptance Checklist:

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

kataggart · 2022-03-03T14:40:36Z

@MichaelCkr Thanks for your work here! We will review and move it through the process.

…ariaDB

MichaelCkr · 2022-03-10T14:15:53Z

I have no clue, why the java 17 integration test failed regarding my changes. I'm quite sure, this is not related, therefore I have done a rebase on the current master.

Rebuild would be nice.

MichaelCkr · 2022-03-18T07:56:17Z

@nvoxland If possible, could you please set the SafeToBuild label again or sth similar to trigger another build?

…reparedStatements' of https://github.com/MichaelCkr/liquibase into 'MichaelCkr-bugfix/#2519_fixed-loadDataChange-for-MySQL-regarding-preparedStatements'

github-actions · 2022-04-20T14:19:05Z

Unit Test Results

  4 512 files ±  0   4 512 suites ±0 35m 11s ⏱️ - 1m 25s
  4 414 tests - 92   4 200 ✔️ - 92   214 💤 ±0 0 ❌ ±0
52 248 runs ±  0 47 240 ✔️ ±  0 5 008 💤 ±0 0 ❌ ±0

Results for commit e5736dd. ± Comparison against base commit a574d1e.

♻️ This comment has been updated with latest results.

nvoxland · 2022-04-20T14:39:50Z

Code review and test results:

It looks like the if statement this PR removes shouldn't have been there. It probably got left from a bad merge, and the performance issue it's trying to address is handled correctly already on line 521. Having the if statement there made it so we didn't load anything which is obviously wrong.

I created #liquibase/liquibase-test-harness#216 to test the change

Things to be aware of

Impacts mysql and mariadb

Things to worry about

Nothing

MichaelCkr · 2022-04-20T15:23:55Z

Thank you very much for adding the test, I commented on your PR.

The failure in the "Run Test Harness" check seems not to be related to my change, is it?

nvoxland · 2022-04-21T22:01:30Z

Correct, @MichaelCkr the failure in the test is not related to your PR. The automation that pulls the correct build of liquibase to test isn't correctly handling PRs from forks. I'm looking into that.

MichaelCkr · 2022-04-22T06:56:23Z

Simple workaround would be, to merge my commit into another bugfix branch within the original liquibase repo (and open another PR from the newly created branch to master).

XDelphiGrl · 2022-05-05T20:02:45Z

@nvoxland, there were MariaDB failures in the test-harness run. Can you take a look, please?

Test Harness Results

…nge-for-MySQL-regarding-preparedStatements'

MichaelCkr · 2022-05-11T19:42:56Z

Note for myself: Test Harness Results

(I'm quite curios, how these checks may run against my branch. :) )

…#2519_fixed-loadDataChange-for-MySQL-regarding-preparedStatements'

…nge-for-MySQL-regarding-preparedStatements' # Conflicts: # liquibase-core/src/main/java/liquibase/change/core/LoadDataChange.java

nvoxland · 2022-05-17T06:03:48Z

@MichaelCkr there is a build job in liquibase/liquibase-test-harness which can pull snapshot builds from liquibase PRs. We trigger that job as part of the PR build process

XDelphiGrl

I've requested changes in the associated liquibase-test-harness PR. When those are done, let me know and I can take a look and finalize my review for this PR.

CC @nvoxland , @MichaelCkr

XDelphiGrl

Fix removes an IF that applied only to MySQL and excluded MariaDB (and others).
Validating test cases added to test harness for MariaDB and MySQL.
- The requested test harness PR changes are complete.
No additional testing required.

APPROVED

Passing Test Harness Execution
Passing Functional Tests

MichaelCkr · 2022-06-13T12:12:53Z

Thanks for adding the Test Harness Tests, the review and the merge.

kataggart added IntegrationMaven ver4.8.0 labels Mar 3, 2022

kataggart added this to To Do in Conditioning++ via automation Mar 3, 2022

kataggart added daily_review and removed daily_review labels Mar 3, 2022

nvoxland added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Mar 8, 2022

liquibase#2519 fixed using preparedStatements on loadData for MySQL/M…

2c0d201

…ariaDB

MichaelCkr force-pushed the bugfix/#2519_fixed-loadDataChange-for-MySQL-regarding-preparedStatements branch from 8a398f5 to 2c0d201 Compare March 10, 2022 14:11

kataggart added ver4.6.2 DBMariaDB OSLinux TypeIncorrectSQL labels Apr 4, 2022

nvoxland added the autocandidate label Apr 13, 2022

Merge branch 'bugfix/#2519_fixed-loadDataChange-for-MySQL-regarding-p…

5147bea

…reparedStatements' of https://github.com/MichaelCkr/liquibase into 'MichaelCkr-bugfix/#2519_fixed-loadDataChange-for-MySQL-regarding-preparedStatements'

nvoxland changed the title ~~#2519 fixed using preparedStatements on loadData for MySQL/MariaDB~~ Fix BLOBs in loadData for MySQL/MariaDB Apr 20, 2022

nvoxland added 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 Apr 20, 2022

nvoxland changed the base branch from master to 1_9 April 20, 2022 14:35

nvoxland changed the base branch from 1_9 to master April 20, 2022 14:36

nvoxland added 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 Apr 20, 2022

nvoxland mentioned this pull request Apr 20, 2022

Test loadData in mariadb with BLOBs liquibase/liquibase-test-harness#216

Merged

nvoxland requested review from suryaaki2 and XDelphiGrl April 20, 2022 14:40

suryaaki2 approved these changes Apr 20, 2022

View reviewed changes

kataggart added the TypeBug label Apr 26, 2022

XDelphiGrl assigned nvoxland May 5, 2022

Merge branch 'master' into 'MichaelCkr-bugfix/#2519_fixed-loadDataCha…

30ecd65

…nge-for-MySQL-regarding-preparedStatements'

nvoxland added 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 May 9, 2022

Merge remote-tracking branch 'origin/master' into 'MichaelCkr-bugfix/…

1a979af

…#2519_fixed-loadDataChange-for-MySQL-regarding-preparedStatements'

nvoxland added 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 May 16, 2022

nvoxland removed their assignment May 16, 2022

Merge branch 'master' into 'MichaelCkr-bugfix/#2519_fixed-loadDataCha…

e5736dd

…nge-for-MySQL-regarding-preparedStatements' # Conflicts: # liquibase-core/src/main/java/liquibase/change/core/LoadDataChange.java

nvoxland added 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 May 17, 2022

XDelphiGrl suggested changes May 18, 2022

View reviewed changes

XDelphiGrl approved these changes May 25, 2022

View reviewed changes

nvoxland merged commit f4561cb into liquibase:master May 25, 2022

Conditioning++ automation moved this from To Do to Done May 25, 2022

nvoxland added this to the NEXT milestone May 25, 2022

nvoxland mentioned this pull request Jun 10, 2022

Liquibase fails to populate mariadb database #2923

Closed

MichaelCkr deleted the bugfix/#2519_fixed-loadDataChange-for-MySQL-regarding-preparedStatements branch June 13, 2022 12:12

pavel-shareiko mentioned this pull request Oct 6, 2022

Liquibase fails to populate PostgreSQL database with blob data jhipster/generator-jhipster#19940

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix BLOBs in loadData for MySQL/MariaDB #2595

Fix BLOBs in loadData for MySQL/MariaDB #2595

MichaelCkr commented Mar 3, 2022 •

edited

kataggart commented Mar 3, 2022

MichaelCkr commented Mar 10, 2022

MichaelCkr commented Mar 18, 2022

github-actions bot commented Apr 20, 2022 •

edited

nvoxland commented Apr 20, 2022

MichaelCkr commented Apr 20, 2022

nvoxland commented Apr 21, 2022

MichaelCkr commented Apr 22, 2022

XDelphiGrl commented May 5, 2022

MichaelCkr commented May 11, 2022 •

edited

nvoxland commented May 17, 2022

XDelphiGrl left a comment

XDelphiGrl left a comment

MichaelCkr commented Jun 13, 2022

Fix BLOBs in loadData for MySQL/MariaDB #2595

Fix BLOBs in loadData for MySQL/MariaDB #2595

Conversation

MichaelCkr commented Mar 3, 2022 • edited

Environment

Pull Request Type

Description

Steps To Reproduce

Actual Behavior

Expected/Desired Behavior

Screenshots (if appropriate)

Additional Context

Fast Track PR Acceptance Checklist:

Need Help?

kataggart commented Mar 3, 2022

MichaelCkr commented Mar 10, 2022

MichaelCkr commented Mar 18, 2022

github-actions bot commented Apr 20, 2022 • edited

Unit Test Results

nvoxland commented Apr 20, 2022

MichaelCkr commented Apr 20, 2022

nvoxland commented Apr 21, 2022

MichaelCkr commented Apr 22, 2022

XDelphiGrl commented May 5, 2022

MichaelCkr commented May 11, 2022 • edited

nvoxland commented May 17, 2022

XDelphiGrl left a comment

Choose a reason for hiding this comment

XDelphiGrl left a comment

Choose a reason for hiding this comment

MichaelCkr commented Jun 13, 2022

MichaelCkr commented Mar 3, 2022 •

edited

github-actions bot commented Apr 20, 2022 •

edited

MichaelCkr commented May 11, 2022 •

edited