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

[DAT-16006] Fixing v8 Checksum calculation issue when using sql dbms attribute #4995

Merged
merged 5 commits into from Oct 9, 2023

Conversation

filipelautert
Copy link
Collaborator

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

Checksums (v8) are calculated differently for XML changesets with the dbms attribute on the change itself (instead of the changeset) after 4.22.0 .

Fixes #4665
Fixes #4457

…es to changesets) - but as we were calculating checksum v9 without them, keep on ignoring them otherwise we will break v9 checksum.
…es to changesets) - but as we were calculating checksum v9 without them, keep on ignoring them otherwise we will break v9 checksum.
…cksum but not v9;

Change from Mysql to H2 improving performance.
if (change.generateStatementsVolatile(database)) {
executor.comment("WARNING The following SQL may change each run and therefore is possibly incorrect and/or invalid:");
}
if ((!(change instanceof DbmsTargetedChange)) || DatabaseList.definitionMatches(((DbmsTargetedChange) change).getDbms(), database, true)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved this check back here (before 4.22.0 it was here and not at DbmsChangeSetFilter)

for (SqlVisitor visitor : changeSet.getSqlVisitors()) {
if (!DatabaseList.definitionMatches(visitor.getApplicableDbms(), database, true)) {
visitorsToRemove.add(visitor);
}
}
for(Change change : changeSet.getChanges()){
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This filtering was moved back to Changeset class

@chadlwilson
Copy link

chadlwilson commented Oct 1, 2023

As noted in #4665 (comment) tested that the artifacts here seem to resolve the issue in #4665 🙏

@sonarcloud
Copy link

sonarcloud bot commented Oct 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@suryaaki2 suryaaki2 merged commit 4f59c96 into master Oct 9, 2023
28 of 30 checks passed
@suryaaki2 suryaaki2 deleted the DAT-16006 branch October 9, 2023 17:13
@filipelautert filipelautert added this to the 1NEXT milestone Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
6 participants