Add include clause to MSSQL forcedid table index (#2659)
* Add include clause to MSSQL forcedid table index * Add changelog * Test fix * Bump MSSQL version
This commit is contained in:
James Agnew
GitHub
parent
b64346ba1e
commit
b31bb945ad
|
|
@ -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."
|
||||
|
|
@ -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<String> myColumns;
|
||||
private Boolean myUnique;
|
||||
private List<String> 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 <myColumns.size(); i++) {
|
||||
mssqlWhereClause += myColumns.get(i) + " IS NOT NULL ";
|
||||
if (i < myColumns.size() - 1) {
|
||||
mssqlWhereClause += "AND ";
|
||||
}
|
||||
}
|
||||
mssqlWhereClause += ")";
|
||||
}
|
||||
String sql = "create " + unique + "index " + myIndexName + " on " + getTableName() + "(" + columns + ")" + mssqlWhereClause;
|
||||
String sql = generateSql();
|
||||
String tableName = getTableName();
|
||||
|
||||
try {
|
||||
|
|
@ -104,6 +94,43 @@ public class AddIndexTask extends BaseTableTask {
|
|||
}
|
||||
}
|
||||
|
||||
@Nonnull
|
||||
String generateSql() {
|
||||
String unique = myUnique ? "unique " : "";
|
||||
String columns = String.join(", ", myColumns);
|
||||
String includeClause = "";
|
||||
String mssqlWhereClause = "";
|
||||
if (!myIncludeColumns.isEmpty()) {
|
||||
switch (getDriverType()) {
|
||||
case POSTGRES_9_4:
|
||||
case MSSQL_2012:
|
||||
includeClause = " INCLUDE (" + StringUtils.join(myIncludeColumns, ", ") + ")";
|
||||
break;
|
||||
case H2_EMBEDDED:
|
||||
case DERBY_EMBEDDED:
|
||||
case MARIADB_10_1:
|
||||
case MYSQL_5_7:
|
||||
case ORACLE_12C:
|
||||
// These platforms don't support the include clause
|
||||
// Per:
|
||||
// https://use-the-index-luke.com/blog/2019-04/include-columns-in-btree-indexes#postgresql-limitations
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (myUnique && getDriverType() == DriverTypeEnum.MSSQL_2012) {
|
||||
mssqlWhereClause = " WHERE (";
|
||||
for (int i = 0; i <myColumns.size(); i++) {
|
||||
mssqlWhereClause += myColumns.get(i) + " IS NOT NULL ";
|
||||
if (i < myColumns.size() - 1) {
|
||||
mssqlWhereClause += "AND ";
|
||||
}
|
||||
}
|
||||
mssqlWhereClause += ")";
|
||||
}
|
||||
String sql = "create " + unique + "index " + myIndexName + " on " + getTableName() + "(" + columns + ")" + includeClause + mssqlWhereClause;
|
||||
return sql;
|
||||
}
|
||||
|
||||
public void setColumns(String... theColumns) {
|
||||
setColumns(Arrays.asList(theColumns));
|
||||
}
|
||||
|
|
@ -126,4 +153,13 @@ public class AddIndexTask extends BaseTableTask {
|
|||
theBuilder.append(myColumns);
|
||||
theBuilder.append(myUnique);
|
||||
}
|
||||
|
||||
public void setIncludeColumns(String... theIncludeColumns) {
|
||||
setIncludeColumns(Arrays.asList(theIncludeColumns));
|
||||
}
|
||||
|
||||
private void setIncludeColumns(List<String> theIncludeColumns) {
|
||||
Validate.notNull(theIncludeColumns);
|
||||
myIncludeColumns = theIncludeColumns;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<ExecutedStatement> myExecutedStatements = new ArrayList<>();
|
||||
private Set<DriverTypeEnum> myOnlyAppliesToPlatforms = new HashSet<>();
|
||||
private boolean myNoColumnShrink;
|
||||
private boolean myFailureAllowed;
|
||||
|
||||
protected BaseTask(String theProductVersion, String theSchemaVersion) {
|
||||
myProductVersion = theProductVersion;
|
||||
mySchemaVersion = theSchemaVersion;
|
||||
}
|
||||
|
||||
public void setOnlyAppliesToPlatforms(Set<DriverTypeEnum> 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;
|
||||
|
|
|
|||
|
|
@ -73,7 +73,19 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks<VersionEnum> {
|
|||
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() {
|
||||
|
|
|
|||
|
|
@ -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<DriverTypeEnum> typesSet = Arrays.stream(theTypes).collect(Collectors.toSet());
|
||||
myTask.setOnlyAppliesToPlatforms(typesSet);
|
||||
return this;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -52,9 +52,11 @@ public abstract class BaseTest {
|
|||
|
||||
@AfterEach
|
||||
public void resetMigrationVersion() throws SQLException {
|
||||
Set<String> 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<String> 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<Supplier<TestDatabaseDetails>> data() {
|
||||
|
|
|
|||
|
|
@ -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<TestDatabaseDetails> theTestDatabaseDetails) {
|
||||
before(theTestDatabaseDetails);
|
||||
|
||||
executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))");
|
||||
|
||||
BaseMigrationTasks<VersionEnum> 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<Map<String, Object>> 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());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue