Oracle create index migration recovery (#5511)

This commit is contained in:
Ken Stevens 2023-11-29 18:15:28 -05:00 committed by GitHub
parent 469873aff7
commit e15d0430d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 86 additions and 3 deletions

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 5511
title: "Previously, when creating an index as a part of a migration, if the index already existed with a different name
on Oracle, the migration would fail. This has been fixed so that the create index migration task now recovers with
a warning message if the index already exists with a different name."

View File

@ -0,0 +1,70 @@
package ca.uhn.fhir.jpa.migrate.taskdef;
import ca.uhn.fhir.jpa.migrate.DriverTypeEnum;
import ca.uhn.test.util.LogbackCaptureTestExtension;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.classic.spi.LoggingEvent;
import oracle.jdbc.OracleDatabaseException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.jdbc.UncategorizedSQLException;
import org.springframework.transaction.TransactionException;
import org.springframework.transaction.support.TransactionTemplate;
import javax.sql.DataSource;
import java.sql.SQLException;
import java.util.Collections;
import java.util.List;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
class AddIndexTaskTest {
@Mock
DriverTypeEnum.ConnectionProperties myConnectionProperties;
@Mock
DataSource myDataSource;
@Mock
TransactionTemplate myTransactionTemplate;
@RegisterExtension
LogbackCaptureTestExtension myLogCapture = new LogbackCaptureTestExtension((Logger) AddIndexTask.ourLog, Level.WARN);
@Test
void testOracleException() throws SQLException {
final AddIndexTask task = new AddIndexTask("1", "1");
task.setColumns(Collections.singletonList("COLUMN_NAME"));
task.setUnique(true);
task.setIndexName("INDEX_NAME");
task.setConnectionProperties(myConnectionProperties);
when(myConnectionProperties.getDataSource()).thenReturn(myDataSource);
when(myConnectionProperties.getTxTemplate()).thenReturn(myTransactionTemplate);
final String sql = "create index INDEX_NAME on TABLE_NAME (COLUMN_NAME)";
when(myTransactionTemplate.execute(any()))
.thenReturn(Collections.emptySet())
.thenThrow(new UncategorizedSQLException("ORA-01408: such column list already indexed", sql, new SQLException("ORA-01408: such column list already indexed", "72000", 1408)));
myLogCapture.clearEvents();
// Red-green: this used to throw an exception. Now it logs a warning.
task.execute();
List<ILoggingEvent> events = myLogCapture.getLogEvents();
assertThat(events, hasSize(1));
LoggingEvent event = (LoggingEvent) events.get(0);
assertThat(event.getFormattedMessage(), containsString("ORA-01408: such column list already indexed"));
}
}

View File

@ -37,7 +37,7 @@ import javax.annotation.Nonnull;
public class AddIndexTask extends BaseTableTask {
private static final Logger ourLog = LoggerFactory.getLogger(AddIndexTask.class);
static final Logger ourLog = LoggerFactory.getLogger(AddIndexTask.class);
private String myIndexName;
private List<String> myColumns;
@ -97,8 +97,15 @@ public class AddIndexTask extends BaseTableTask {
try {
executeSql(tableName, sql);
} catch (Exception e) {
if (e.toString().contains("already exists")) {
ourLog.warn("Index {} already exists", myIndexName);
String message = e.toString();
if (message.contains("already exists")
||
// The Oracle message is ORA-01408: such column list already indexed
// TODO KHS consider db-specific handling here that uses the error code instead of the message so
// this is language independent
// e.g. if the db is Oracle than checking e.getErrorCode() == 1408 should detect this case
message.contains("already indexed")) {
ourLog.warn("Index {} already exists: {}", myIndexName, e.getMessage());
} else {
throw e;
}