From 67f9f4ce587cca13580c6c73ae8873265052927a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sun, 1 Oct 2023 18:35:35 +0200 Subject: [PATCH 1/3] Support rollback for formatted SQL changelogs Fixes #361 --- .../addColumnWithRollbackSqlChangelog/pom.xml | 76 +++++++++++++++++++ .../test-changelog.sql | 14 ++++ .../verify.groovy | 42 ++++++++++ .../PerconaFormattedSqlChangeLogParser.java | 34 +++++++-- .../ext/percona/ChangeLogParserTest.java | 23 +++++- .../ext/percona/changelog/test-changelog.sql | 3 + 6 files changed, 181 insertions(+), 11 deletions(-) create mode 100644 src/it/addColumnWithRollbackSqlChangelog/pom.xml create mode 100644 src/it/addColumnWithRollbackSqlChangelog/test-changelog.sql create mode 100644 src/it/addColumnWithRollbackSqlChangelog/verify.groovy diff --git a/src/it/addColumnWithRollbackSqlChangelog/pom.xml b/src/it/addColumnWithRollbackSqlChangelog/pom.xml new file mode 100644 index 00000000..953b9d26 --- /dev/null +++ b/src/it/addColumnWithRollbackSqlChangelog/pom.xml @@ -0,0 +1,76 @@ + + + + + + 4.0.0 + @project.groupId@.it + liquibase-percona-it-addColumnWithRollbackSqlChangelog + 1.0-SNAPSHOT + my-app + + + + + org.liquibase + liquibase-maven-plugin + @liquibase.version@ + + + @project.groupId@ + @project.artifactId@ + @project.version@ + + + mysql + mysql-connector-java + @mysql.version@ + + + + test-changelog.sql + com.mysql.jdbc.Driver + jdbc:mysql://@config_host@:@config_port@/@config_dbname@?useSSL=false&allowPublicKeyRetrieval=true + @config_user@ + @config_password@ + + + + update + pre-integration-test + + update + + + + rollbackSQL + pre-integration-test + + rollbackSQL + + + 3 + + + + rollback + pre-integration-test + + rollback + + + 3 + + + + + + + diff --git a/src/it/addColumnWithRollbackSqlChangelog/test-changelog.sql b/src/it/addColumnWithRollbackSqlChangelog/test-changelog.sql new file mode 100644 index 00000000..d12291ce --- /dev/null +++ b/src/it/addColumnWithRollbackSqlChangelog/test-changelog.sql @@ -0,0 +1,14 @@ +--liquibase formatted sql + +--changeset Alice:1 +--rollback DROP TABLE person; +CREATE TABLE person (name VARCHAR(255) NOT NULL, CONSTRAINT PK_PERSON PRIMARY KEY (name)); + +--changeset Alice:2 +--rollback ALTER TABLE person DROP COLUMN address; +ALTER TABLE person ADD address VARCHAR(255) NULL; + +--changeset Alice:3 +--liquibasePercona:usePercona="false" +--rollback ALTER TABLE person DROP COLUMN email; +ALTER TABLE person ADD email VARCHAR(255) NULL; diff --git a/src/it/addColumnWithRollbackSqlChangelog/verify.groovy b/src/it/addColumnWithRollbackSqlChangelog/verify.groovy new file mode 100644 index 00000000..73d23438 --- /dev/null +++ b/src/it/addColumnWithRollbackSqlChangelog/verify.groovy @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +File buildLog = new File( basedir, 'build.log' ) +assert buildLog.exists() +def buildLogText = buildLog.text; + +// changeset 3: email has usePercona="false" +assert buildLogText.contains("Rolling Back Changeset: test-changelog.sql::3::Alice") +assert !buildLogText.contains("Executing: pt-online-schema-change --alter-foreign-keys-method=auto --nocheck-unique-key-change --alter=\"DROP COLUMN email\" --password=*** --execute h=${config_host},P=${config_port},u=${config_user},D=testdb,t=person"); +// changeset 2: address +assert buildLogText.contains("Rolling Back Changeset: test-changelog.sql::2::Alice") +assert buildLogText.contains("Executing: pt-online-schema-change --alter-foreign-keys-method=auto --nocheck-unique-key-change --alter=\"DROP COLUMN address\" --password=*** --execute h=${config_host},P=${config_port},u=${config_user},D=testdb,t=person"); +// changeset 1: table +assert buildLogText.contains("Rolling Back Changeset: test-changelog.sql::1::Alice") + + +File sql = new File( basedir, 'target/liquibase/migrate.sql' ) +assert sql.exists() +def sqlText = sql.text; +assert sqlText.contains("pt-online-schema-change") +// changeset 3: email has usePercona="false" +assert !sqlText.contains("-- pt-online-schema-change --alter-foreign-keys-method=auto --nocheck-unique-key-change --alter=\"DROP COLUMN email\" --password=*** --execute h=${config_host},P=${config_port},u=${config_user},D=testdb,t=person;"); +// changeset 2: address +assert sqlText.contains("-- pt-online-schema-change --alter-foreign-keys-method=auto --nocheck-unique-key-change --alter=\"DROP COLUMN address\" --password=*** --execute h=${config_host},P=${config_port},u=${config_user},D=testdb,t=person;"); +assert !sqlText.contains("password=${config_password}") diff --git a/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java b/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java index d3b2aec9..454d2677 100644 --- a/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java +++ b/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java @@ -57,14 +57,7 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame for (Change change : changeSet.getChanges()) { RawSQLChange rawSQLChange = (RawSQLChange) change; - PerconaRawSQLChange perconaChange = new PerconaRawSQLChange(); - perconaChange.setSql(rawSQLChange.getSql()); - perconaChange.setSplitStatements(rawSQLChange.isSplitStatements()); - perconaChange.setStripComments(rawSQLChange.isStripComments()); - perconaChange.setEndDelimiter(rawSQLChange.getEndDelimiter()); - perconaChange.setRerunnable(rawSQLChange.isRerunnable()); - perconaChange.setComment(rawSQLChange.getComment()); - perconaChange.setDbms(rawSQLChange.getDbms()); + PerconaRawSQLChange perconaChange = convert(rawSQLChange); perconaChangeSet.addChange(perconaChange); String sql = perconaChange.getSql(); @@ -78,9 +71,34 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame perconaChange.setPerconaOptions(perconaOptionsMatcher.group(1)); } } + + for (Change change : changeSet.getRollback().getChanges()) { + RawSQLChange rawSQLChange = (RawSQLChange) change; + PerconaRawSQLChange rollbackChange = convert(rawSQLChange); + + assert perconaChangeSet.getChanges().size() == 1; + PerconaRawSQLChange forwardChange = (PerconaRawSQLChange) perconaChangeSet.getChanges().get(0); + rollbackChange.setUsePercona(forwardChange.getUsePercona()); + rollbackChange.setPerconaOptions(forwardChange.getPerconaOptions()); + + perconaChangeSet.getRollback().getChanges().add(rollbackChange); + } + changeLog.getChangeSets().set(i, perconaChangeSet); } return changeLog; } + + private static PerconaRawSQLChange convert(RawSQLChange rawSQLChange) { + PerconaRawSQLChange perconaChange = new PerconaRawSQLChange(); + perconaChange.setSql(rawSQLChange.getSql()); + perconaChange.setSplitStatements(rawSQLChange.isSplitStatements()); + perconaChange.setStripComments(rawSQLChange.isStripComments()); + perconaChange.setEndDelimiter(rawSQLChange.getEndDelimiter()); + perconaChange.setRerunnable(rawSQLChange.isRerunnable()); + perconaChange.setComment(rawSQLChange.getComment()); + perconaChange.setDbms(rawSQLChange.getDbms()); + return perconaChange; + } } diff --git a/src/test/java/liquibase/ext/percona/ChangeLogParserTest.java b/src/test/java/liquibase/ext/percona/ChangeLogParserTest.java index e93d1bff..51f157b2 100644 --- a/src/test/java/liquibase/ext/percona/ChangeLogParserTest.java +++ b/src/test/java/liquibase/ext/percona/ChangeLogParserTest.java @@ -23,6 +23,7 @@ import liquibase.change.Change; import liquibase.changelog.ChangeLogParameters; +import liquibase.changelog.ChangeSet; import liquibase.changelog.DatabaseChangeLog; import liquibase.parser.ChangeLogParser; import liquibase.parser.ChangeLogParserFactory; @@ -78,11 +79,27 @@ public void testReadLiquibaseUsePerconaFlagXML() throws Exception { public void testReadLiquibaseUsePerconaFlagSQL() throws Exception { DatabaseChangeLog changelog = loadChangeLog("test-changelog.sql"); Assertions.assertEquals(3, changelog.getChangeSets().size()); - Change change = changelog.getChangeSets().get(0).getChanges().get(0); + + // changeset 1 + ChangeSet changeSet = changelog.getChangeSets().get(0); + Change change = changeSet.getChanges().get(0); assertChange(change, PerconaRawSQLChange.class, null, null); - change = changelog.getChangeSets().get(1).getChanges().get(0); + Assertions.assertEquals(1, changeSet.getRollback().getChanges().size()); + Change rollback = changeSet.getRollback().getChanges().get(0); + assertChange(rollback, PerconaRawSQLChange.class, null, null); + + // changeset 2 + changeSet = changelog.getChangeSets().get(1); + change = changeSet.getChanges().get(0); assertChange(change, PerconaRawSQLChange.class, Boolean.FALSE, null); - change = changelog.getChangeSets().get(2).getChanges().get(0); + rollback = changeSet.getRollback().getChanges().get(0); + assertChange(rollback, PerconaRawSQLChange.class, Boolean.FALSE, null); + + // changeset 3 + changeSet = changelog.getChangeSets().get(2); + change = changeSet.getChanges().get(0); assertChange(change, PerconaRawSQLChange.class, null, "--foo"); + rollback = changeSet.getRollback().getChanges().get(0); + assertChange(rollback, PerconaRawSQLChange.class, null, "--foo"); } } diff --git a/src/test/resources/liquibase/ext/percona/changelog/test-changelog.sql b/src/test/resources/liquibase/ext/percona/changelog/test-changelog.sql index c48a5fc4..5c6a5da5 100644 --- a/src/test/resources/liquibase/ext/percona/changelog/test-changelog.sql +++ b/src/test/resources/liquibase/ext/percona/changelog/test-changelog.sql @@ -1,12 +1,15 @@ --liquibase formatted sql --changeset Alice:1 +--rollback DROP TABLE person; CREATE TABLE person (name VARCHAR(255) NOT NULL, CONSTRAINT PK_PERSON PRIMARY KEY (name)); --changeset Alice:2 --liquibasePercona:usePercona="false" +--rollback ALTER TABLE person DROP COLUMN address; ALTER TABLE person ADD address VARCHAR(255) NULL; --changeset Alice:3 -- liquibasePercona:perconaOptions="--foo" +--rollback ALTER TABLE person DROP COLUMN email; ALTER TABLE person ADD email VARCHAR(255) NULL; From 93c2a30138df7510a99d676802869453255c37b0 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 5 Oct 2023 14:19:47 +0200 Subject: [PATCH 2/3] Improve logging --- .../PerconaFormattedSqlChangeLogParser.java | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java b/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java index 454d2677..de5f368b 100644 --- a/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java +++ b/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java @@ -18,17 +18,21 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import liquibase.Scope; import liquibase.change.Change; import liquibase.change.core.RawSQLChange; import liquibase.changelog.ChangeLogParameters; import liquibase.changelog.ChangeSet; import liquibase.changelog.DatabaseChangeLog; import liquibase.exception.ChangeLogParseException; +import liquibase.logging.Logger; import liquibase.parser.core.formattedsql.FormattedSqlChangeLogParser; import liquibase.resource.ResourceAccessor; import liquibase.servicelocator.PrioritizedService; public class PerconaFormattedSqlChangeLogParser extends FormattedSqlChangeLogParser { + private static Logger log = Scope.getCurrentScope().getLog(PerconaFormattedSqlChangeLogParser.class); + @Override public int getPriority() { return PrioritizedService.PRIORITY_DEFAULT + 50; @@ -55,6 +59,11 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame changeSet.getRunWith(), changeSet.getRunWithSpoolFile(), changeSet.isRunInTransaction(), changeSet.getObjectQuotingStrategy(), changeLog); + log.fine(String.format("Changeset %s::%s::%s contains %d changes and %d rollback changes", + changeSet.getFilePath(), changeSet.getId(), changeSet.getAuthor(), + changeSet.getChanges().size(), + changeSet.getRollback().getChanges().size())); + for (Change change : changeSet.getChanges()) { RawSQLChange rawSQLChange = (RawSQLChange) change; PerconaRawSQLChange perconaChange = convert(rawSQLChange); @@ -76,10 +85,22 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame RawSQLChange rawSQLChange = (RawSQLChange) change; PerconaRawSQLChange rollbackChange = convert(rawSQLChange); - assert perconaChangeSet.getChanges().size() == 1; - PerconaRawSQLChange forwardChange = (PerconaRawSQLChange) perconaChangeSet.getChanges().get(0); - rollbackChange.setUsePercona(forwardChange.getUsePercona()); - rollbackChange.setPerconaOptions(forwardChange.getPerconaOptions()); + if (!perconaChangeSet.getChanges().isEmpty()) { + if (perconaChangeSet.getChanges().size() > 1) { + log.warning(String.format("The changeset %s::%s::%s contains %d changes - using the first " + + "one to copy percona options", + perconaChangeSet.getFilePath(), perconaChangeSet.getId(), perconaChangeSet.getAuthor(), + perconaChangeSet.getChanges().size())); + } + + PerconaRawSQLChange forwardChange = (PerconaRawSQLChange) perconaChangeSet.getChanges().get(0); + rollbackChange.setUsePercona(forwardChange.getUsePercona()); + rollbackChange.setPerconaOptions(forwardChange.getPerconaOptions()); + } else { + log.warning(String.format("The changeset %s::%s::%s contains 0 changes, but contains rollback - " + + "using default percona options", + perconaChangeSet.getFilePath(), perconaChangeSet.getId(), perconaChangeSet.getAuthor())); + } perconaChangeSet.getRollback().getChanges().add(rollbackChange); } From b3542fc0d029f8a87f4ebd95d68765210471fd2f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 5 Oct 2023 15:12:25 +0200 Subject: [PATCH 3/3] Make regex patterns static final --- .../PerconaFormattedSqlChangeLogParser.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java b/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java index de5f368b..fe068118 100644 --- a/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java +++ b/src/main/java/liquibase/ext/percona/PerconaFormattedSqlChangeLogParser.java @@ -31,7 +31,10 @@ import liquibase.servicelocator.PrioritizedService; public class PerconaFormattedSqlChangeLogParser extends FormattedSqlChangeLogParser { - private static Logger log = Scope.getCurrentScope().getLog(PerconaFormattedSqlChangeLogParser.class); + private static final Logger LOG = Scope.getCurrentScope().getLog(PerconaFormattedSqlChangeLogParser.class); + + private static final Pattern USE_PERCONA_PATTERN = Pattern.compile("(?im)^\\s*\\-\\-\\s*liquibasePercona:usePercona=\"(false|true)\"\\s*$"); + private static final Pattern PERCONA_OPTIONS_PATTERN = Pattern.compile("(?im)^\\s*\\-\\-\\s*liquibasePercona:perconaOptions=\"(.*)\"\\s*$"); @Override public int getPriority() { @@ -40,9 +43,6 @@ public int getPriority() { @Override public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParameters changeLogParameters, ResourceAccessor resourceAccessor) throws ChangeLogParseException { - Pattern usePerconaPattern = Pattern.compile("(?im)^\\s*\\-\\-\\s*liquibasePercona:usePercona=\"(false|true)\"\\s*$"); - Pattern perconaOptionsPattern = Pattern.compile("(?im)^\\s*\\-\\-\\s*liquibasePercona:perconaOptions=\"(.*)\"\\s*$"); - DatabaseChangeLog changeLog = super.parse(physicalChangeLogLocation, changeLogParameters, resourceAccessor); for (int i = 0; i < changeLog.getChangeSets().size(); i++) { @@ -59,7 +59,7 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame changeSet.getRunWith(), changeSet.getRunWithSpoolFile(), changeSet.isRunInTransaction(), changeSet.getObjectQuotingStrategy(), changeLog); - log.fine(String.format("Changeset %s::%s::%s contains %d changes and %d rollback changes", + LOG.fine(String.format("Changeset %s::%s::%s contains %d changes and %d rollback changes", changeSet.getFilePath(), changeSet.getId(), changeSet.getAuthor(), changeSet.getChanges().size(), changeSet.getRollback().getChanges().size())); @@ -70,12 +70,12 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame perconaChangeSet.addChange(perconaChange); String sql = perconaChange.getSql(); - Matcher usePerconaMatcher = usePerconaPattern.matcher(sql); + Matcher usePerconaMatcher = USE_PERCONA_PATTERN.matcher(sql); if (usePerconaMatcher.find()) { perconaChange.setUsePercona(Boolean.valueOf(usePerconaMatcher.group(1))); } - Matcher perconaOptionsMatcher = perconaOptionsPattern.matcher(sql); + Matcher perconaOptionsMatcher = PERCONA_OPTIONS_PATTERN.matcher(sql); if (perconaOptionsMatcher.find()) { perconaChange.setPerconaOptions(perconaOptionsMatcher.group(1)); } @@ -87,7 +87,7 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame if (!perconaChangeSet.getChanges().isEmpty()) { if (perconaChangeSet.getChanges().size() > 1) { - log.warning(String.format("The changeset %s::%s::%s contains %d changes - using the first " + + LOG.warning(String.format("The changeset %s::%s::%s contains %d changes - using the first " + "one to copy percona options", perconaChangeSet.getFilePath(), perconaChangeSet.getId(), perconaChangeSet.getAuthor(), perconaChangeSet.getChanges().size())); @@ -97,7 +97,7 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame rollbackChange.setUsePercona(forwardChange.getUsePercona()); rollbackChange.setPerconaOptions(forwardChange.getPerconaOptions()); } else { - log.warning(String.format("The changeset %s::%s::%s contains 0 changes, but contains rollback - " + + LOG.warning(String.format("The changeset %s::%s::%s contains 0 changes, but contains rollback - " + "using default percona options", perconaChangeSet.getFilePath(), perconaChangeSet.getId(), perconaChangeSet.getAuthor())); }