From 35e3317b3251725cae2a0228c7398b766a308ebf Mon Sep 17 00:00:00 2001 From: "philipp.kleber" Date: Thu, 28 Nov 2024 20:35:05 +0100 Subject: [PATCH 1/5] Implement support for hibernate timestamp columns with the 'with time zone' suffix. --- .../ext/hibernate/snapshot/ColumnSnapshotGenerator.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java b/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java index 777e6630..c2f29975 100644 --- a/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java +++ b/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java @@ -183,7 +183,11 @@ protected DataType toDataType(String hibernateType, Integer sqlTypeCode) throws if (!matcher.matches()) { return null; } - DataType dataType = new DataType(matcher.group(1)); + String typeName = matcher.group(1); + if (hibernateType.endsWith("with time zone")) { + typeName += " with timezone"; + } + DataType dataType = new DataType(typeName); if (matcher.group(3).isEmpty()) { if (!matcher.group(2).isEmpty()) { dataType.setColumnSize(Integer.parseInt(matcher.group(2))); @@ -200,6 +204,8 @@ protected DataType toDataType(String hibernateType, Integer sqlTypeCode) throws } } + Scope.getCurrentScope().getLog(getClass()).info("Converted column data type - hibernate type: " + hibernateType + ", SQL type: " + sqlTypeCode + ", type name: " + typeName); + dataType.setDataTypeId(sqlTypeCode); return dataType; } From 5d9b1e6aa31fc085a3b0a033ed94ae1975cd8b09 Mon Sep 17 00:00:00 2001 From: "philipp.kleber" Date: Wed, 11 Dec 2024 18:00:05 +0100 Subject: [PATCH 2/5] Add unit test for timestamp columns with or without timezones --- src/test/java/com/example/timezone/Item.java | 51 ++++++++++++++ .../snapshot/TimezoneSnapshotTest.java | 66 +++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 src/test/java/com/example/timezone/Item.java create mode 100644 src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java diff --git a/src/test/java/com/example/timezone/Item.java b/src/test/java/com/example/timezone/Item.java new file mode 100644 index 00000000..0d421091 --- /dev/null +++ b/src/test/java/com/example/timezone/Item.java @@ -0,0 +1,51 @@ +package com.example.timezone; + +import jakarta.persistence.*; + +import java.time.Instant; +import java.time.LocalDateTime; + +@Entity +public class Item { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private long id; + + @Column + private Instant timestamp1; + + @Column + private LocalDateTime timestamp2; + + @Column(columnDefinition = "timestamp") + private Instant timestamp3; + + @Column(columnDefinition = "TIMESTAMP WITH TIME ZONE") + private LocalDateTime timestamp4; + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public Instant getTimestamp1() { + return timestamp1; + } + + public void setTimestamp1(Instant timestamp1) { + this.timestamp1 = timestamp1; + } + + public LocalDateTime getTimestamp2() { + return timestamp2; + } + + public void setTimestamp2(LocalDateTime timestamp2) { + this.timestamp2 = timestamp2; + } + +} diff --git a/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java b/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java new file mode 100644 index 00000000..656c79aa --- /dev/null +++ b/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java @@ -0,0 +1,66 @@ +package liquibase.ext.hibernate.snapshot; + +import liquibase.CatalogAndSchema; +import liquibase.database.Database; +import liquibase.integration.commandline.CommandLineUtils; +import liquibase.resource.ClassLoaderResourceAccessor; +import liquibase.snapshot.DatabaseSnapshot; +import liquibase.snapshot.SnapshotControl; +import liquibase.snapshot.SnapshotGeneratorFactory; +import liquibase.structure.DatabaseObject; +import liquibase.structure.core.Column; +import liquibase.structure.core.DataType; +import org.hamcrest.FeatureMatcher; +import org.hamcrest.Matcher; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class TimezoneSnapshotTest { + + @Test + public void testTimezoneColumns() throws Exception { + Database database = CommandLineUtils.createDatabaseObject(new ClassLoaderResourceAccessor(this.getClass().getClassLoader()), "hibernate:spring:com.example.timezone?dialect=org.hibernate.dialect.H2Dialect", null, null, null, null, null, false, false, null, null, null, null, null, null, null); + + DatabaseSnapshot snapshot = SnapshotGeneratorFactory.getInstance().createSnapshot(CatalogAndSchema.DEFAULT, database, new SnapshotControl(database)); + + assertThat( + snapshot.get(Column.class), + hasItems( + // Instant colum should result in 'timestamp with timezone' type + allOf( + hasProperty("name", equalTo("timestamp1")), + hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalTo("timestamp with timezone"))) + ), + // LocalDateTime colum should result in 'timestamp' type + allOf( + hasProperty("name", equalTo("timestamp2")), + hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalTo("timestamp"))) + ), + // Colum with explicit definition + allOf( + hasProperty("name", equalTo("timestamp3")), + hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalTo("timestamp"))) + ), + // Colum with explicit definition + allOf( + hasProperty("name", equalTo("timestamp4")), + hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalToIgnoringCase("timestamp with timezone"))) + ) + ) + ); + } + + private static FeatureMatcher hasDatabaseAttribute(String attribute, Class type, Matcher matcher) { + return new FeatureMatcher<>(matcher, attribute, attribute) { + + @Override + protected T featureValueOf(DatabaseObject databaseObject) { + return databaseObject.getAttribute(attribute, type); + } + + }; + } + +} From 80da062a324500d1e8dc106aab8bb8e99c75e072 Mon Sep 17 00:00:00 2001 From: "philipp.kleber" Date: Wed, 11 Dec 2024 18:01:57 +0100 Subject: [PATCH 3/5] Fix timezone handling for timestamp columns with an explicit columnDefinition --- .../snapshot/ColumnSnapshotGenerator.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java b/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java index c2f29975..74e443f1 100644 --- a/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java +++ b/src/main/java/liquibase/ext/hibernate/snapshot/ColumnSnapshotGenerator.java @@ -34,6 +34,9 @@ */ public class ColumnSnapshotGenerator extends HibernateSnapshotGenerator { + private static final String SQL_TIMEZONE_SUFFIX = "with time zone"; + private static final String LIQUIBASE_TIMEZONE_SUFFIX = "with timezone"; + private final static Pattern pattern = Pattern.compile("([^\\(]*)\\s*\\(?\\s*(\\d*)?\\s*,?\\s*(\\d*)?\\s*([^\\(]*?)\\)?"); public ColumnSnapshotGenerator() { @@ -183,10 +186,23 @@ protected DataType toDataType(String hibernateType, Integer sqlTypeCode) throws if (!matcher.matches()) { return null; } + String typeName = matcher.group(1); - if (hibernateType.endsWith("with time zone")) { - typeName += " with timezone"; + + // Liquibase seems to use 'with timezone' instead of 'with time zone', + // so we remove any 'with time zone' suffixes here. + // The corresponding 'with timezone' suffix will then be added below, + // because in that case hibernateType also ends with 'with time zone'. + if (typeName.toLowerCase().endsWith(SQL_TIMEZONE_SUFFIX)) { + typeName = typeName.substring(0, typeName.length() - SQL_TIMEZONE_SUFFIX.length()).stripTrailing(); } + + // If hibernateType ends with 'with time zone' we need to add the corresponding + // 'with timezone' suffix to the Liquibase type. + if (hibernateType.toLowerCase().endsWith(SQL_TIMEZONE_SUFFIX)) { + typeName += (" " + LIQUIBASE_TIMEZONE_SUFFIX); + } + DataType dataType = new DataType(typeName); if (matcher.group(3).isEmpty()) { if (!matcher.group(2).isEmpty()) { From a5bc00d2b8ea1a00af8be05f8f58af63d9ae05c4 Mon Sep 17 00:00:00 2001 From: "philipp.kleber" Date: Wed, 11 Dec 2024 18:07:10 +0100 Subject: [PATCH 4/5] Improve comments on timezone unit test --- .../ext/hibernate/snapshot/TimezoneSnapshotTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java b/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java index 656c79aa..615f3dc7 100644 --- a/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java +++ b/src/test/java/liquibase/ext/hibernate/snapshot/TimezoneSnapshotTest.java @@ -28,22 +28,22 @@ public void testTimezoneColumns() throws Exception { assertThat( snapshot.get(Column.class), hasItems( - // Instant colum should result in 'timestamp with timezone' type + // Instant column should result in 'timestamp with timezone' type allOf( hasProperty("name", equalTo("timestamp1")), hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalTo("timestamp with timezone"))) ), - // LocalDateTime colum should result in 'timestamp' type + // LocalDateTime column should result in 'timestamp' type allOf( hasProperty("name", equalTo("timestamp2")), hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalTo("timestamp"))) ), - // Colum with explicit definition + // Instant column with explicit definition 'timestamp' should result in 'timestamp' type allOf( hasProperty("name", equalTo("timestamp3")), hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalTo("timestamp"))) ), - // Colum with explicit definition + // LocalDateTime Colum with explicit definition 'TIMESTAMP WITH TIME ZONE' should result in 'TIMESTAMP with timezone' type allOf( hasProperty("name", equalTo("timestamp4")), hasDatabaseAttribute("type", DataType.class, hasProperty("typeName", equalToIgnoringCase("timestamp with timezone"))) From c617c314b8c2f2acbcaac61e296fd441ba2e1b3a Mon Sep 17 00:00:00 2001 From: "philipp.kleber" Date: Thu, 12 Dec 2024 12:49:10 +0100 Subject: [PATCH 5/5] Add missing getters and setters to timestamp test entity --- src/test/java/com/example/timezone/Item.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/java/com/example/timezone/Item.java b/src/test/java/com/example/timezone/Item.java index 0d421091..8d9aacc6 100644 --- a/src/test/java/com/example/timezone/Item.java +++ b/src/test/java/com/example/timezone/Item.java @@ -48,4 +48,20 @@ public void setTimestamp2(LocalDateTime timestamp2) { this.timestamp2 = timestamp2; } + public Instant getTimestamp3() { + return timestamp3; + } + + public void setTimestamp3(Instant timestamp3) { + this.timestamp3 = timestamp3; + } + + public LocalDateTime getTimestamp4() { + return timestamp4; + } + + public void setTimestamp4(LocalDateTime timestamp4) { + this.timestamp4 = timestamp4; + } + }