diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2659-add-include-clause-to-mssql-forcedid.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2659-add-include-clause-to-mssql-forcedid.yaml new file mode 100644 index 00000000000..1561581f3bf --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2659-add-include-clause-to-mssql-forcedid.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 2659 +title: "The MSSQL-specific index definition for the ForcedId table in the JPA server has been enhanced + to include an INCLUDE() clause, which should significantly improve performance." diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java index 5dc129fe262..c00aa23e008 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTask.java @@ -29,8 +29,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.thymeleaf.util.StringUtils; +import javax.annotation.Nonnull; import java.sql.SQLException; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Set; @@ -41,6 +43,7 @@ public class AddIndexTask extends BaseTableTask { private String myIndexName; private List myColumns; private Boolean myUnique; + private List myIncludeColumns = Collections.emptyList(); public AddIndexTask(String theProductVersion, String theSchemaVersion) { super(theProductVersion, theSchemaVersion); @@ -77,20 +80,7 @@ public class AddIndexTask extends BaseTableTask { logInfo(ourLog, "Going to add a {} index named {} on table {} for columns {}", (myUnique ? "UNIQUE" : "NON-UNIQUE"), myIndexName, getTableName(), myColumns); - String unique = myUnique ? "unique " : ""; - String columns = String.join(", ", myColumns); - String mssqlWhereClause = ""; - if (myUnique && getDriverType() == DriverTypeEnum.MSSQL_2012) { - mssqlWhereClause = " WHERE ("; - for (int i = 0; i theIncludeColumns) { + Validate.notNull(theIncludeColumns); + myIncludeColumns = theIncludeColumns; + } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java index cbbcb824d10..38dd2b331b9 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.migrate.taskdef; */ import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; +import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.intellij.lang.annotations.Language; @@ -34,7 +35,9 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -52,14 +55,19 @@ public abstract class BaseTask { private boolean myDryRun; private boolean myDoNothing; private List myExecutedStatements = new ArrayList<>(); + private Set myOnlyAppliesToPlatforms = new HashSet<>(); private boolean myNoColumnShrink; private boolean myFailureAllowed; - protected BaseTask(String theProductVersion, String theSchemaVersion) { myProductVersion = theProductVersion; mySchemaVersion = theSchemaVersion; } + public void setOnlyAppliesToPlatforms(Set theOnlyAppliesToPlatforms) { + Validate.notNull(theOnlyAppliesToPlatforms); + myOnlyAppliesToPlatforms = theOnlyAppliesToPlatforms; + } + public String getProductVersion() { return myProductVersion; } @@ -91,7 +99,6 @@ public abstract class BaseTask { return myDescription; } - @SuppressWarnings("unchecked") public BaseTask setDescription(String theDescription) { myDescription = theDescription; return this; @@ -176,19 +183,30 @@ public abstract class BaseTask { ourLog.info("Skipping stubbed task: {}", getDescription()); return; } + if (!myOnlyAppliesToPlatforms.isEmpty()) { + if (!myOnlyAppliesToPlatforms.contains(getDriverType())) { + ourLog.debug("Skipping task {} as it does not apply to {}", getDescription(), getDriverType()); + return; + } + } + if (!myOnlyAppliesToPlatforms.isEmpty()) { + if (!myOnlyAppliesToPlatforms.contains(getDriverType())) { + ourLog.debug("Skipping task {} as it does not apply to {}", getDescription(), getDriverType()); + return; + } + } doExecute(); } protected abstract void doExecute() throws SQLException; - public void setFailureAllowed(boolean theFailureAllowed) { - myFailureAllowed = theFailureAllowed; - } - protected boolean isFailureAllowed() { return myFailureAllowed; } + public void setFailureAllowed(boolean theFailureAllowed) { + myFailureAllowed = theFailureAllowed; + } public String getFlywayVersion() { String releasePart = myProductVersion; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index cf9bd6a3bf7..1ae55d348b6 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -73,7 +73,19 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { init510(); // 20200516 - 20201028 init520(); // 20201029 - init530(); - init540(); // 20210218 - + init540(); // 20210218 - 20210520 + init550(); // 20210520 - + } + + private void init550() { + + Builder version = forVersion(VersionEnum.V5_5_0); + + // For MSSQL only - Replace ForcedId index with a version that has an INCLUDE clause + Builder.BuilderWithTableName forcedId = version.onTable("HFJ_FORCED_ID"); + forcedId.dropIndex("20210516.1", "IDX_FORCEDID_TYPE_FID").onlyAppliesToPlatforms(DriverTypeEnum.MSSQL_2012); + forcedId.addIndex("20210516.2", "IDX_FORCEDID_TYPE_FID").unique(true).includeColumns("RESOURCE_PID").withColumns("RESOURCE_TYPE", "FORCED_ID").onlyAppliesToPlatforms(DriverTypeEnum.MSSQL_2012); + } private void init540() { diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java index 6fecd36e110..bf4a49868b0 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java @@ -27,6 +27,8 @@ import org.intellij.lang.annotations.Language; import java.util.Arrays; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; public class Builder { @@ -291,6 +293,7 @@ public class Builder { public class BuilderAddIndexUnique { private final String myVersion; private final boolean myUnique; + private String[] myIncludeColumns; public BuilderAddIndexUnique(String theVersion, boolean theUnique) { myVersion = theVersion; @@ -313,9 +316,17 @@ public class Builder { task.setUnique(myUnique); task.setColumns(theColumnNames); task.setDoNothing(theDoNothing); + if (myIncludeColumns != null) { + task.setIncludeColumns(myIncludeColumns); + } addTask(task); return task; } + + public BuilderAddIndexUnique includeColumns(String... theIncludeColumns) { + myIncludeColumns = theIncludeColumns; + return this; + } } } @@ -487,6 +498,12 @@ public class Builder { return this; } + public BuilderCompleteTask onlyAppliesToPlatforms(DriverTypeEnum... theTypes) { + Set typesSet = Arrays.stream(theTypes).collect(Collectors.toSet()); + myTask.setOnlyAppliesToPlatforms(typesSet); + return this; + } + } } diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java index 97457960566..58c3ddd7667 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.migrate.taskdef; +import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; import ca.uhn.fhir.jpa.migrate.JdbcUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -11,6 +12,7 @@ import java.util.function.Supplier; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasItem; +import static org.junit.jupiter.api.Assertions.assertEquals; public class AddIndexTest extends BaseTest { @@ -107,4 +109,25 @@ public class AddIndexTest extends BaseTest { assertThat(JdbcUtils.getIndexNames(getConnectionProperties(), "SOMETABLE"), containsInAnyOrder("IDX_DIFINDEX", "IDX_ANINDEX")); } + @Test + public void testIncludeColumns() { + + AddIndexTask task = new AddIndexTask("1", "1"); + task.setIndexName("IDX_ANINDEX"); + task.setTableName("SOMETABLE"); + task.setColumns("PID", "TEXTCOL"); + task.setIncludeColumns("FOO1", "FOO2"); + task.setUnique(false); + + // MSSQL supports include clause + task.setDriverType(DriverTypeEnum.MSSQL_2012); + String sql = task.generateSql(); + assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL) INCLUDE (FOO1, FOO2)", sql); + + // Oracle does not support include clause + task.setDriverType(DriverTypeEnum.ORACLE_12C); + sql = task.generateSql(); + assertEquals("create index IDX_ANINDEX on SOMETABLE(PID, TEXTCOL)", sql); + } + } diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java index d6d49f427ec..777df41e6a1 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java @@ -52,9 +52,11 @@ public abstract class BaseTest { @AfterEach public void resetMigrationVersion() throws SQLException { - Set tableNames = JdbcUtils.getTableNames(getConnectionProperties()); - if (tableNames.contains(SchemaMigrator.HAPI_FHIR_MIGRATION_TABLENAME)) { - executeSql("DELETE from " + SchemaMigrator.HAPI_FHIR_MIGRATION_TABLENAME + " where \"installed_rank\" > 0"); + if (getConnectionProperties() != null) { + Set tableNames = JdbcUtils.getTableNames(getConnectionProperties()); + if (tableNames.contains(SchemaMigrator.HAPI_FHIR_MIGRATION_TABLENAME)) { + executeSql("DELETE from " + SchemaMigrator.HAPI_FHIR_MIGRATION_TABLENAME + " where \"installed_rank\" > 0"); + } } } @@ -77,7 +79,9 @@ public abstract class BaseTest { @AfterEach public void after() { - myConnectionProperties.close(); + if (myConnectionProperties != null) { + myConnectionProperties.close(); + } } protected DriverTypeEnum getDriverType() { @@ -98,6 +102,10 @@ public abstract class BaseTest { myMigrator = theMigrator; } + public DriverTypeEnum getDriverType() { + return myConnectionProperties.getDriverType(); + } + } public static Stream> data() { diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java index 18d21bc6529..7e0dc9d2804 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java @@ -1,8 +1,8 @@ package ca.uhn.fhir.jpa.migrate.taskdef; +import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; import ca.uhn.fhir.jpa.migrate.tasks.api.BaseMigrationTasks; import ca.uhn.fhir.util.VersionEnum; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -55,4 +55,31 @@ public class ExecuteRawSqlTaskTest extends BaseTest { assertEquals(0, output.size()); } + + @ParameterizedTest(name = "{index}: {0}") + @MethodSource("data") + public void testOnlyAppliesToPlatforms(Supplier theTestDatabaseDetails) { + before(theTestDatabaseDetails); + + executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))"); + + BaseMigrationTasks tasks = new BaseMigrationTasks<>(); + tasks + .forVersion(VersionEnum.V4_0_0) + .executeRawSql("2001.01", "INSERT INTO SOMETABLE (PID, TEXTCOL) VALUES (123, 'abc')") + .onlyAppliesToPlatforms(DriverTypeEnum.H2_EMBEDDED); + + getMigrator().addTasks(tasks.getTasks(VersionEnum.V0_1, VersionEnum.V4_0_0)); + getMigrator().migrate(); + + List> output = executeQuery("SELECT PID FROM SOMETABLE"); + if (theTestDatabaseDetails.get().getDriverType() == DriverTypeEnum.H2_EMBEDDED) { + assertEquals(1, output.size()); + assertEquals(123L, output.get(0).get("PID")); + } else { + assertEquals(0, output.size()); + } + } + + } diff --git a/pom.xml b/pom.xml index 421284b017d..10caafb9add 100644 --- a/pom.xml +++ b/pom.xml @@ -1105,7 +1105,7 @@ com.microsoft.sqlserver mssql-jdbc - 7.4.1.jre8 + 9.2.1.jre8 javax.mail