From b27b4e7038856c52584506e23b86be25cd6031ff Mon Sep 17 00:00:00 2001 From: "robin.bygrave" Date: Thu, 25 May 2023 20:00:36 +1200 Subject: [PATCH] Postgres DDL - Don't use cast when just changing varchar column size --- .../ddlgeneration/platform/PostgresDdl.java | 21 ++++++++--- .../platform/PlatformDdl_AlterColumnTest.java | 2 +- .../platform/PostgresDdlTest.java | 35 +++++++++++++++++++ .../dbmigration/postgres/1.1.sql | 2 +- .../dbmigration/postgres/1.3.sql | 2 +- .../postgres/idx_postgres.migrations | 4 +-- .../dbmigration/postgres9/1.1.sql | 2 +- .../dbmigration/postgres9/1.3.sql | 2 +- .../postgres9/idx_postgres.migrations | 4 +-- .../dbmigration/yugabyte/1.1.sql | 2 +- .../dbmigration/yugabyte/1.3.sql | 2 +- .../yugabyte/idx_yugabyte.migrations | 4 +-- 12 files changed, 65 insertions(+), 17 deletions(-) diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdl.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdl.java index e7567f90e..9296ef4c9 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdl.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdl.java @@ -9,7 +9,7 @@ import io.ebeaninternal.dbmigration.migration.Column; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; +import java.util.regex.Pattern; import static java.util.stream.Collectors.toList; @@ -18,6 +18,8 @@ import static java.util.stream.Collectors.toList; */ public class PostgresDdl extends PlatformDdl { + private static final Pattern PLAIN_VARCHAR = Pattern.compile("(varchar\\()(\\d+)(\\))"); + private static final String dropIndexConcurrentlyIfExists = "drop index concurrently if exists "; public PostgresDdl(DatabasePlatform platform) { @@ -74,9 +76,20 @@ public class PostgresDdl extends PlatformDdl { @Override protected void alterColumnType(DdlWrite writer, AlterColumn alter) { String type = convert(alter.getType()); - alterTable(writer, alter.getTableName()).append(alterColumn, alter.getColumnName()) - .append(columnSetType).append(type) - .append(" using ").append(alter.getColumnName()).append("::").append(type); + var alterTable = alterTable(writer, alter.getTableName()) + .append(alterColumn, alter.getColumnName()) + .append(columnSetType).append(type); + if (useCast(type, alter.getCurrentType())) { + alterTable.append(" using ").append(alter.getColumnName()).append("::").append(type); + } + } + + static boolean useCast(String newType, String currentType) { + return currentType == null || !isPlainVarchar(newType) || !isPlainVarchar(currentType); + } + + static boolean isPlainVarchar(String type) { + return PLAIN_VARCHAR.matcher(type).matches(); } @Override diff --git a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl_AlterColumnTest.java b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl_AlterColumnTest.java index 9804e0c43..21c1320ab 100644 --- a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl_AlterColumnTest.java +++ b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl_AlterColumnTest.java @@ -107,7 +107,7 @@ public class PlatformDdl_AlterColumnTest { sql = alterColumn(pgDdl, alter); softly.assertThat(sql).isEqualTo("-- apply alter tables\n" - + "alter table mytab alter column acol type varchar(50) using acol::varchar(50);\n" + + "alter table mytab alter column acol type varchar(50);\n" + "alter table mytab alter column acol set default 'hi';\n" + "alter table mytab alter column acol set not null;\n"); diff --git a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdlTest.java b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdlTest.java index 607f8166b..1ab246798 100644 --- a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdlTest.java +++ b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PostgresDdlTest.java @@ -13,6 +13,41 @@ class PostgresDdlTest { final PostgresDdl postgresDdl = new PostgresDdl(new PostgresPlatform()); + @Test + void useCast_notWhenSimpleVarcharLengthChange() { + assertThat(PostgresDdl.useCast("varchar(10)", "varchar(150)")).isFalse(); + } + + @Test + void useCast() { + assertThat(PostgresDdl.useCast("varchar", "varchar(150)")).isTrue(); + assertThat(PostgresDdl.useCast("varchar(1)", "varchar")).isTrue(); + assertThat(PostgresDdl.useCast("varchar(1)[]", "varchar(2)[]")).isTrue(); + } + + @Test + void useCastTrue() { + assertThat(PostgresDdl.useCast("text(10)", "text(150)")).isTrue(); + assertThat(PostgresDdl.useCast("number(5)", "number(10)")).isTrue(); + assertThat(PostgresDdl.useCast("number(5,3)", "number(10,2)")).isTrue(); + } + + @Test + void isPlainVarchar() { + assertThat(PostgresDdl.isPlainVarchar("varchar(1)")).isTrue(); + assertThat(PostgresDdl.isPlainVarchar("varchar(10)")).isTrue(); + assertThat(PostgresDdl.isPlainVarchar("varchar(150)")).isTrue(); + } + + @Test + void isPlainVarchar_false() { + assertThat(PostgresDdl.isPlainVarchar("varcha(10)")).isFalse(); + assertThat(PostgresDdl.isPlainVarchar("text(10)")).isFalse(); + assertThat(PostgresDdl.isPlainVarchar("number(10)")).isFalse(); + assertThat(PostgresDdl.isPlainVarchar("varchar(10)[]")).isFalse(); + assertThat(PostgresDdl.isPlainVarchar("varchar")).isFalse(); + } + @Test void setLockTimeout() { final String sql = postgresDdl.setLockTimeout(5); diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.1.sql b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.1.sql index c8273e6e4..b27d6abfb 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.1.sql +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.1.sql @@ -86,7 +86,7 @@ alter table migtest_ckey_detail add column two_key varchar(127); alter table migtest_ckey_parent add column assoc_id integer; alter table migtest_e_basic alter column status set default 'A'; alter table migtest_e_basic alter column status set not null; -alter table migtest_e_basic alter column status2 type varchar(127) using status2::varchar(127); +alter table migtest_e_basic alter column status2 type varchar(127); alter table migtest_e_basic alter column status2 drop default; alter table migtest_e_basic alter column status2 drop not null; alter table migtest_e_basic alter column a_lob drop default; diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.3.sql b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.3.sql index e9a619dcd..c0033e76b 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.3.sql +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/1.3.sql @@ -68,7 +68,7 @@ update migtest_e_history6 set test_number2 = 7 where test_number2 is null; -- apply alter tables alter table migtest_e_basic alter column status drop default; alter table migtest_e_basic alter column status drop not null; -alter table migtest_e_basic alter column status2 type varchar(1) using status2::varchar(1); +alter table migtest_e_basic alter column status2 type varchar(1); alter table migtest_e_basic alter column status2 set default 'N'; alter table migtest_e_basic alter column status2 set not null; alter table migtest_e_basic alter column a_lob type varchar(255) using a_lob::varchar(255); diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/idx_postgres.migrations b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/idx_postgres.migrations index 59e6a24ce..b84026676 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/idx_postgres.migrations +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres/idx_postgres.migrations @@ -1,7 +1,7 @@ -73982981, 1.0__initial.sql --1076179036, 1.1.sql +-167317190, 1.1.sql 274267648, 1.2__dropsFor_1.1.sql --634032267, 1.3.sql +-42523216, 1.3.sql -709144111, 1.4__dropsFor_1.3.sql 783227075, R__multi_comments.sql 561281075, R__order_views.sql diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.1.sql b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.1.sql index 5cc39012b..95e602bea 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.1.sql +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.1.sql @@ -86,7 +86,7 @@ alter table migtest_ckey_detail add column two_key varchar(127); alter table migtest_ckey_parent add column assoc_id integer; alter table migtest_e_basic alter column status set default 'A'; alter table migtest_e_basic alter column status set not null; -alter table migtest_e_basic alter column status2 type varchar(127) using status2::varchar(127); +alter table migtest_e_basic alter column status2 type varchar(127); alter table migtest_e_basic alter column status2 drop default; alter table migtest_e_basic alter column status2 drop not null; alter table migtest_e_basic alter column a_lob drop default; diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.3.sql b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.3.sql index 5b92e11b8..764f42f09 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.3.sql +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/1.3.sql @@ -68,7 +68,7 @@ update migtest_e_history6 set test_number2 = 7 where test_number2 is null; -- apply alter tables alter table migtest_e_basic alter column status drop default; alter table migtest_e_basic alter column status drop not null; -alter table migtest_e_basic alter column status2 type varchar(1) using status2::varchar(1); +alter table migtest_e_basic alter column status2 type varchar(1); alter table migtest_e_basic alter column status2 set default 'N'; alter table migtest_e_basic alter column status2 set not null; alter table migtest_e_basic alter column a_lob type varchar(255) using a_lob::varchar(255); diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/idx_postgres.migrations b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/idx_postgres.migrations index e64bb01b0..b8d809633 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/idx_postgres.migrations +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/postgres9/idx_postgres.migrations @@ -1,7 +1,7 @@ -1033742102, 1.0__initial.sql --1861202915, 1.1.sql +382308009, 1.1.sql 274267648, 1.2__dropsFor_1.1.sql -601885195, 1.3.sql +2062205426, 1.3.sql -709144111, 1.4__dropsFor_1.3.sql 783227075, R__multi_comments.sql 561281075, R__order_views.sql diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.1.sql b/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.1.sql index 1714a3346..37a18e7e6 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.1.sql +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.1.sql @@ -81,7 +81,7 @@ alter table migtest_ckey_detail add column two_key varchar(127); alter table migtest_ckey_parent add column assoc_id integer; alter table migtest_e_basic alter column status set default 'A'; alter table migtest_e_basic alter column status set not null; -alter table migtest_e_basic alter column status2 type varchar(127) using status2::varchar(127); +alter table migtest_e_basic alter column status2 type varchar(127); alter table migtest_e_basic alter column status2 drop default; alter table migtest_e_basic alter column status2 drop not null; alter table migtest_e_basic alter column a_lob drop default; diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.3.sql b/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.3.sql index 748634b8c..0ea0b770c 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.3.sql +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/1.3.sql @@ -68,7 +68,7 @@ update migtest_e_history6 set test_number2 = 7 where test_number2 is null; -- apply alter tables alter table migtest_e_basic alter column status drop default; alter table migtest_e_basic alter column status drop not null; -alter table migtest_e_basic alter column status2 type varchar(1) using status2::varchar(1); +alter table migtest_e_basic alter column status2 type varchar(1); alter table migtest_e_basic alter column status2 set default 'N'; alter table migtest_e_basic alter column status2 set not null; alter table migtest_e_basic alter column a_lob type varchar(255) using a_lob::varchar(255); diff --git a/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/idx_yugabyte.migrations b/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/idx_yugabyte.migrations index 99c68d769..6f3fe9620 100644 --- a/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/idx_yugabyte.migrations +++ b/ebean-test/src/test/resources/migrationtest/dbmigration/yugabyte/idx_yugabyte.migrations @@ -1,7 +1,7 @@ 113365976, 1.0__initial.sql -637070078, 1.1.sql +716377547, 1.1.sql 274267648, 1.2__dropsFor_1.1.sql --1383541363, 1.3.sql +-1802768940, 1.3.sql -709144111, 1.4__dropsFor_1.3.sql 561281075, R__order_views.sql