Merge pull request #2565 from IBM/issue-2564

fhir-persistance-schema-cli update-schema not updating functions. #2564
LinuxForHealth · Jun 30, 2021 · ced088f · ced088f
2 parents 4b1cfac + a6b5fef
commit ced088f
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 19 deletions.
diff --git a/...atabase-utils/src/main/java/com/ibm/fhir/database/utils/common/CommonDatabaseAdapter.java b/...atabase-utils/src/main/java/com/ibm/fhir/database/utils/common/CommonDatabaseAdapter.java
@@ -443,6 +443,7 @@ public void updateTenantStatus(String adminSchemaName, int tenantId, TenantStatu
      * @param schemaName
      * @param newTenantId
      */
+    @Override
     public void addNewTenantPartitions(Collection<Table> tables, String schemaName, int newTenantId) {
         // NOP for all databases except Db2
     }

diff --git a/fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/model/BaseObject.java b/fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/model/BaseObject.java
@@ -263,9 +263,15 @@ public void applyVersion(IDatabaseAdapter target, IVersionHistoryService vhs) {
             // Apply this change to the target database
             apply(vhs.getVersion(getSchemaName(), getObjectType().name(), getObjectName()), target);
 
-            // call back to the version history service to add the new version to the table
-            // being used to track the change history
-            vhs.addVersion(getSchemaName(), getObjectType().name(), getObjectName(), getVersion());
+            // Check if the PROCEDURE is this exact version (Applies to FunctionDef and ProcedureDef)
+            if (DatabaseObjectType.PROCEDURE.equals(getObjectType())
+                    && !vhs.applies(getSchemaName(), getObjectType().name(), getObjectName(), version)) {
+                logger.info("Version History is already current, refreshing the definition " + getVersion() + " " + vhs.getVersion(schemaName, objectName, objectName));
+            } else {
+                // call back to the version history service to add the new version to the table
+                // being used to track the change history
+                vhs.addVersion(getSchemaName(), getObjectType().name(), getObjectName(), getVersion());
+            }
         }
     }
 

diff --git a/fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/model/FunctionDef.java b/fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/model/FunctionDef.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2020
+ * (C) Copyright IBM Corp. 2019, 2021
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -44,7 +44,7 @@ public void apply(Integer priorVersion, IDatabaseAdapter target) {
             logger.warning("Found '" + migrations.size() + "' migration steps, but performing 'create or replace' instead");
         }
 
-        // Procedures are applied with "Create or replace", so just do a regular apply
+        // Functions are applied with "Create or replace", so just do a regular apply
         apply(target);
     }
 
@@ -58,17 +58,11 @@ protected void grantGroupPrivileges(IDatabaseAdapter target, Set<Privilege> grou
         target.grantFunctionPrivileges(getSchemaName(), getObjectName(), group, toUser);
     }
 
-    /* (non-Javadoc)
-     * @see com.ibm.fhir.database.utils.model.IDatabaseObject#visit(com.ibm.fhir.database.utils.model.DataModelVisitor)
-     */
     @Override
     public void visit(DataModelVisitor v) {
         v.visited(this);
     }
 
-    /* (non-Javadoc)
-     * @see com.ibm.fhir.database.utils.model.IDatabaseObject#visitReverse(com.ibm.fhir.database.utils.model.DataModelVisitor)
-     */
     @Override
     public void visitReverse(DataModelVisitor v) {
         v.visited(this);

diff --git a/fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/postgres/PostgresAdapter.java b/fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/postgres/PostgresAdapter.java
@@ -8,7 +8,9 @@
 
 import java.sql.Connection;
 import java.sql.PreparedStatement;
+import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
@@ -413,16 +415,41 @@ public void createOrReplaceFunction(String schemaName, String functionName, Supp
         final String objectName = DataDefinitionUtil.getQualifiedName(schemaName, functionName);
         logger.info("Dropping current function " + objectName);
 
-        final StringBuilder ddl = new StringBuilder()
-                .append("DROP FUNCTION IF EXISTS ")
-                .append(objectName);
-
-        final String ddlString = ddl.toString();
-        if (logger.isLoggable(Level.FINE)) {
-            logger.fine(ddlString);
+        final String DROP_SPECIFIC =
+                "select p.oid::regproc || '(' || pg_get_function_identity_arguments(p.oid) || ')' " +
+                "    FROM pg_catalog.pg_proc p" +
+                "    WHERE p.oid::regproc::text = LOWER(?)";
+
+        List<String> existingFunctions = new ArrayList<>();
+        if (connectionProvider != null) {
+            try (Connection c = connectionProvider.getConnection()) {
+                try (PreparedStatement p = c.prepareStatement(DROP_SPECIFIC)) {
+                    p.setString(1, objectName);
+                    if (p.execute()) {
+                        // Closes with PreparedStatement
+                        ResultSet rs = p.getResultSet();
+                        while (rs.next()) {
+                            existingFunctions.add(rs.getString(1));
+                        }
+                    }
+                }
+            } catch (SQLException x) {
+                throw getTranslator().translate(x);
+            }
         }
-        runStatement(ddlString);
 
+        // As the signatures are mutated, we don't want to be in the situation where the signature change, and
+        // we can't drop.
+        for (String existingFunction : existingFunctions) {
+            final StringBuilder ddl = new StringBuilder()
+                    .append("DROP FUNCTION ")
+                    .append(existingFunction);
+            try {
+                runStatement(ddl.toString());
+            } catch (UndefinedNameException x) {
+                logger.warning(ddl + "; PROCEDURE not found");
+            }
+        }
         super.createOrReplaceFunction(schemaName, functionName, supplier);
     }
 }