Skip to content

Commit

Permalink
Avoid creating unnecessary tables for Resource and DomainResource
Browse files Browse the repository at this point in the history
resource types #713

- Checks for DOMAINRESOURCE and RESOURCE tables view
- Removes the artifacts
- Bonus: added fix for null condition on a code in
DefaultMemberMatchStrategy

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
  • Loading branch information
prb112 committed Nov 5, 2021
1 parent a4cece3 commit f94fb7f
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2561,6 +2561,7 @@ private void applyTableRemovalForV0021() {
logger.info("V0021 Migration: Removing Abstract Tables from schema '" + schemaName + "'");
MigrateV0021AbstractTypeRemoval cmd = new MigrateV0021AbstractTypeRemoval(adapter, adminSchemaName, schemaName, forceUnusedTableRemoval);
adapter.runStatement(cmd);
logger.info("V0021 Migration: Completed the Removal of the Abstract Tables from schema '" + schemaName + "'");
}
} catch (DataAccessException x) {
// Something went wrong, so mark the transaction as failed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import com.ibm.fhir.database.utils.api.DataAccessException;
import com.ibm.fhir.database.utils.api.IDatabaseAdapter;
Expand All @@ -34,6 +35,19 @@ public class MigrateV0021AbstractTypeRemoval implements IDatabaseStatement {

private static final Logger LOG = Logger.getLogger(MigrateV0021AbstractTypeRemoval.class.getName());

private static final List<String> VALUE_TYPES = Arrays.asList(
"RESOURCE_TOKEN_REFS",
"DATE_VALUES",
"LATLNG_VALUES",
"NUMBER_VALUES",
"QUANTITY_VALUES",
"STR_VALUES",
"PROFILES",
"TAGS",
"SECURITY");

private static final List<String> VALUE_TYPES_LOWER = VALUE_TYPES.stream().map(s -> s.toLowerCase()).collect(Collectors.toList());

// The Adapter
private final IDatabaseAdapter adapter;

Expand Down Expand Up @@ -171,30 +185,16 @@ private void checkDataTables(IDatabaseTranslator translator, Connection c) {
*/
private void removeBaseArtifacts(IDatabaseTranslator translator, Connection c) {
List<String> tables = Arrays.asList("DOMAINRESOURCE_", "RESOURCE_");
List<String> valueTypes = Arrays.asList(
"DATE_VALUES",
"LATLNG_VALUES",
"NUMBER_VALUES",
"QUANTITY_VALUES",
"RESOURCE_TOKEN_REFS",
"STR_VALUES",
"PROFILES",
"TAGS",
"SECURITY");


// Run across both tables
for (String tablePrefix : tables) {
// Drop the Values Tables
for (String valueType : valueTypes) {
adapter.dropTable(schemaName, tablePrefix + valueType);
}

// Drop the supporting tables
adapter.dropTable(schemaName, tablePrefix + "logical_resources");
adapter.dropTable(schemaName, tablePrefix + "resources");

// Drop the View for the Table
adapter.dropView(schemaName, tablePrefix + "_token_values_V");
if (translator.getType() == DbType.POSTGRESQL) {
runDropTableResourceGroup(translator, c, schemaName.toLowerCase(), tablePrefix.toLowerCase(), VALUE_TYPES_LOWER);
} else {
runDropTableResourceGroup(translator, c, schemaName, tablePrefix, VALUE_TYPES);
}
}

// Pattern for multitenant is to prefix tables with DRP_
Expand All @@ -206,8 +206,34 @@ private void removeBaseArtifacts(IDatabaseTranslator translator, Connection c) {
// Drop the tables, when the tables don't exists the errors are swallowed
// and logs print warnings saying the tables don't exist. That's OK.
for (String deprecatedTable : UnusedTableRemovalNeedsV0021Migration.DEPRECATED_TABLES) {
adapter.dropTable(schemaName, prefix + deprecatedTable);
String table = prefix + deprecatedTable;
if (translator.getType() == DbType.POSTGRESQL) {
adapter.dropTable(schemaName.toLowerCase(), table.toLowerCase());
} else {
adapter.dropTable(schemaName, table);
}
}
}

/**
* run drop table resource group
* @param translator
* @param c
* @param schemaName
* @param tablePrefix
* @param valueTypes
*/
public void runDropTableResourceGroup(IDatabaseTranslator translator, Connection c, String schemaName, String tablePrefix, List<String> valueTypes) {
adapter.dropView(schemaName, tablePrefix + "token_values_v");

// Drop the Values Tables
for (String valueType : valueTypes) {
adapter.dropTable(schemaName, tablePrefix + valueType);
}

// Drop the supporting tables
adapter.dropTable(schemaName, tablePrefix + "logical_resources");
adapter.dropTable(schemaName, tablePrefix + "resources");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.ibm.fhir.database.utils.api.IDatabaseSupplier;
import com.ibm.fhir.database.utils.api.IDatabaseTranslator;
import com.ibm.fhir.database.utils.model.DbType;

/**
* Checks to see if any of the tables exist in the target database.
Expand Down Expand Up @@ -87,7 +88,7 @@ public Boolean run(IDatabaseTranslator translator, Connection c) {
* @return
*/
public boolean checkDb2(IDatabaseTranslator translator, Connection c) {
final String sql = "SELECT tabname FROM SYSCAT.TABLES TABS"
final String sql = "SELECT 1 FROM SYSCAT.TABLES TABS"
+ " WHERE TABS.tabschema = ? "
+ " AND TABS.tabname in ( " + addParameterMarkers(TABLE_COUNT) + " )";
return hasTables(translator, c, sql);
Expand All @@ -103,7 +104,7 @@ public boolean checkDb2(IDatabaseTranslator translator, Connection c) {
public boolean checkDerby(IDatabaseTranslator translator, Connection c) {
// Grab the list of tables for the configured schema from the Derby sys catalog
final String sql = ""
+ "SELECT tables.tablename FROM sys.systables AS tables "
+ "SELECT 1 FROM sys.systables AS tables "
+ " JOIN sys.sysschemas AS schemas "
+ " ON (tables.schemaid = schemas.schemaid) "
+ " WHERE schemas.schemaname = ?"
Expand All @@ -122,8 +123,8 @@ public boolean checkPostgres(IDatabaseTranslator translator, Connection c) {
// Grab the list of tables for the configured schema from the PostgreSQL schema
// catalog
final String sql = ""
+ "SELECT table_name FROM information_schema.tables "
+ " WHERE table_schema = ?"
+ "SELECT 1 FROM information_schema.tables "
+ " WHERE table_schema = lower(?)"
+ " AND table_name in (" + addParameterMarkers(TABLE_COUNT) + ")";
return hasTables(translator, c, sql);
}
Expand Down Expand Up @@ -157,7 +158,11 @@ private boolean hasTables(IDatabaseTranslator translator, Connection c, final St
ps.setString(i++, schemaName);

for (String deprecatedTable : DEPRECATED_TABLES) {
ps.setString(i++, deprecatedTable);
if (translator.getType() == DbType.POSTGRESQL) {
ps.setString(i++, deprecatedTable.toLowerCase());
} else {
ps.setString(i++, deprecatedTable);
}
}
ResultSet rs = ps.executeQuery();
return rs.next();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;

import java.sql.Connection;
Expand All @@ -31,7 +30,6 @@
import com.ibm.fhir.database.utils.model.PhysicalDataModel;
import com.ibm.fhir.database.utils.pool.PoolConnectionProvider;
import com.ibm.fhir.database.utils.transaction.SimpleTransactionProvider;
import com.ibm.fhir.database.utils.version.CreateVersionHistory;
import com.ibm.fhir.database.utils.version.CreateWholeSchemaVersion;
import com.ibm.fhir.database.utils.version.VersionHistoryService;
import com.ibm.fhir.schema.app.Main;
Expand Down Expand Up @@ -62,7 +60,7 @@ public void testFhirSchema() throws Exception {
try (DerbyFhirDatabase db = new DerbyFhirDatabase(DB_NAME)) {
System.out.println("FHIR database exists.");
checkDatabase(db, db.getSchemaName());

// Test the schema drop
testDrop(db, db.getSchemaName());
}
Expand All @@ -87,9 +85,9 @@ protected void testDrop(IConnectionProvider cp, String schemaName) throws SQLExc
FhirSchemaGenerator gen = new FhirSchemaGenerator(ADMIN_SCHEMA_NAME, schemaName, false);
gen.buildSchema(pdm);
pdm.drop(adapter, FhirSchemaGenerator.SCHEMA_GROUP_TAG, FhirSchemaGenerator.FHIRDATA_GROUP);

CreateWholeSchemaVersion.dropTable(schemaName, adapter);

// Check that the schema is empty
List<SchemaInfoObject> schemaObjects = adapter.listSchemaObjects(schemaName);
boolean schemaIsEmpty = schemaObjects.isEmpty();
Expand All @@ -100,7 +98,7 @@ protected void testDrop(IConnectionProvider cp, String schemaName) throws SQLExc
}
assertTrue(schemaIsEmpty);
}

// Now we're all done we can finally clean up the version-history for
// our schema. Make sure we don't get a version for an object we know
// we created
Expand Down Expand Up @@ -137,10 +135,10 @@ protected void checkDatabase(IConnectionProvider cp, String schemaName) throws S
JdbcTarget tgt = new JdbcTarget(c);
DerbyAdapter adapter = new DerbyAdapter(tgt);
checkRefSequence(adapter);

// Check that we have the correct number of tables. This will need to be updated
// whenever tables, views or sequences are added or removed
assertEquals(adapter.listSchemaObjects(schemaName).size(), 1943);
assertEquals(adapter.listSchemaObjects(schemaName).size(), 1917);
c.commit();
} catch (Throwable t) {
c.rollback();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* SPDX-License-Identifier: Apache-2.0
*/

package com.ibm.fhir.schema.derby;

import static org.testng.Assert.assertEquals;
Expand All @@ -27,7 +27,7 @@
public class DerbySchemaVersionsTest {
private static final String TARGET_DIR = "target/derby/";
private static final String SCHEMA_NAME = "APP";

@Test
public void test() throws Exception {
String dbPath = TARGET_DIR + "fhirdb";
Expand All @@ -41,18 +41,18 @@ public void test() throws Exception {
PoolConnectionProvider connectionPool = new PoolConnectionProvider(cp, 10);
ITransactionProvider transactionProvider = new SimpleTransactionProvider(connectionPool);
SchemaVersionsManager svm = new SchemaVersionsManager(db.getTranslator(), connectionPool, transactionProvider, SCHEMA_NAME);

svm.updateSchemaVersionId(FhirSchemaVersion.V0001);
assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0001.vid());
svm.updateSchemaVersionId(FhirSchemaVersion.V0002);
assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0002.vid());
svm.updateSchemaVersionId(FhirSchemaVersion.V0003);
assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0003.vid());

// Make sure we can correctly determine the latest schema version value
svm.updateSchemaVersion();
assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0020.vid());
assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0021.vid());

assertTrue(svm.isLatestSchema());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,9 @@ private void addIdValue(Identifier identifier) {
// If there is more than one Coding it has to be MB.
for (Coding coding : identifier.getType().getCoding()) {
if (coding.getSystem() != null
&& "http://terminology.hl7.org/CodeSystem/v2-0203"
&& "http://terminology.hl7.org/CodeSystem/v2-0203"
.equals(coding.getSystem().getValue())
&& coding.getCode() != null
&& "MB".equals(coding.getCode().getValue())) {
// We only want to extract the code system v2-0203 with MB
Uri sys = identifier.getSystem();
Expand Down

0 comments on commit f94fb7f

Please sign in to comment.