diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java index 20b0338137c..82998d43626 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java @@ -41,6 +41,9 @@ public class JDBCConstants { public static final String ESCAPE_UNDERSCORE = ESCAPE_CHAR + "_"; public static final String ESCAPE_PERCENT = ESCAPE_CHAR + PERCENT_WILDCARD; public static final String ESCAPE_EXPR = " ESCAPE '" + ESCAPE_CHAR + "'"; + public static final String WHEN = " WHEN "; + public static final String THEN = " THEN "; + public static final String END = " END"; /** * Maps Parameter modifiers to SQL operators. diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java index 1eba8cd9a23..dd7320ad6ad 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java @@ -118,19 +118,18 @@ List search(String sqlSelect) * @throws FHIRPersistenceDataAccessException * @throws FHIRPersistenceDBConnectException */ - List searchForIds(String sqlSelect) - throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; + List searchForIds(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; /** * Searches for Resources that contain one of the passed ids. + * @param resourceType - The type of the FHIR Resource * @param resourceIds - A List of resource ids. * @return List - A List of resources matching the the passed list of ids. * @throws FHIRPersistenceDataAccessException * @throws FHIRPersistenceDBConnectException */ - List searchByIds(List resourceIds) - throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; - + List searchByIds(String resourceType, List resourceIds) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; + /** * Executes a count query based on the data contained in the passed SqlQueryData, using it's encapsulated search string and bind variables. * @param queryData - Contains a search string and (optionally) bind variables. @@ -173,27 +172,6 @@ List searchByIds(List resourceIds) * @throws FHIRPersistenceDataAccessException */ Integer readResourceTypeId(String parameterName) throws FHIRPersistenceDBConnectException, FHIRPersistenceDataAccessException; - - /** - * This method supports the execution of a specialized query designed to return Resource ids, based on the contents - * of the passed select statement. - * Note that the first column to be selected MUST be the Resource.id column. - * @param sqlSelect - A select for Resource ids. - * @return - A List of resource ids that satisfy the passed SQL query. - * @throws FHIRPersistenceDataAccessException - * @throws FHIRPersistenceDBConnectException - */ - List searchForIds(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; - - /** - * Searches for Resources that contain one of the passed ids. - * @param resourceType - The type of the FHIR Resource. - * @param resourceIds - A List of resource ids. - * @return List - A List of resources matching the the passed list of ids. - * @throws FHIRPersistenceDataAccessException - * @throws FHIRPersistenceDBConnectException - */ - List searchByIds(String resourceType, List resourceIds) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; /** * Adds a resource type / resource id pair to a candidate collection for population into the ResourceTypesCache. diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ResourceDAOImpl.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ResourceDAOImpl.java index 4483aa32b4b..9e20cc2106e 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ResourceDAOImpl.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ResourceDAOImpl.java @@ -6,6 +6,8 @@ package com.ibm.fhir.persistence.jdbc.dao.impl; +import static com.ibm.fhir.persistence.jdbc.JDBCConstants.*; + import java.sql.CallableStatement; import java.sql.Connection; import java.sql.PreparedStatement; @@ -90,6 +92,8 @@ public class ResourceDAOImpl extends FHIRDbDAOImpl implements ResourceDAO { "FROM %s_RESOURCES R, %s_LOGICAL_RESOURCES LR WHERE R.LOGICAL_RESOURCE_ID = LR.LOGICAL_RESOURCE_ID AND " + "R.RESOURCE_ID IN "; + private static final String SQL_ORDER_BY_IDS = "ORDER BY CASE R.RESOURCE_ID "; + private static final String DERBY_PAGINATION_PARMS = "OFFSET ? ROWS FETCH NEXT ? ROWS ONLY"; private static final String DB2_PAGINATION_PARMS = "LIMIT ? OFFSET ?"; @@ -418,64 +422,7 @@ public List searchForIds(SqlQueryData queryData) throws FHIRPersistenceDat return resourceIds; } - @Override - public List searchByIds(String resourceType, List resourceIds) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { - final String METHODNAME = "searchByIds"; - log.entering(CLASSNAME, METHODNAME); - - if (resourceIds.isEmpty()) { - return Collections.emptyList(); - } - - Connection connection = null; - PreparedStatement stmt = null; - ResultSet resultSet = null; - String errMsg; - StringBuilder idQuery = new StringBuilder(); - List resources = new ArrayList<>(); - String stmtString = null; - long dbCallStartTime; - double dbCallDuration; - - try { - stmtString = String.format(this.getSearchByIdsSql(resourceType)); - idQuery.append(stmtString); - idQuery.append("("); - for (int i = 0; i < resourceIds.size(); i++) { - if (i > 0) { - idQuery.append(","); - } - idQuery.append(resourceIds.get(i)); - } - idQuery.append(")"); - - connection = this.getConnection(); - stmt = connection.prepareStatement(idQuery.toString()); - dbCallStartTime = System.nanoTime(); - resultSet = stmt.executeQuery(); - dbCallDuration = (System.nanoTime()-dbCallStartTime)/1e6; - if (log.isLoggable(Level.FINE)) { - log.fine("DB search by ids complete. SQL=[" + idQuery + "] executionTime=" + dbCallDuration + "ms"); - } - resources = this.createDTOs(resultSet); - } catch(FHIRPersistenceException e) { - throw e; - } catch (Throwable e) { - FHIRPersistenceDataAccessException fx = new FHIRPersistenceDataAccessException("Failure retrieving FHIR Resources"); - errMsg = "Failure retrieving FHIR Resources. SQL=[" + idQuery + "]"; - throw severe(log, fx, errMsg, e); - } finally { - this.cleanup(resultSet, stmt, connection); - log.exiting(CLASSNAME, METHODNAME); - } - return resources; - } - - protected String getSearchByIdsSql(String resourceType) { - String stmtString; - stmtString = String.format(SQL_SEARCH_BY_IDS, resourceType, resourceType); - return stmtString; - } + /** * Adds a resource type/ resource id pair to a candidate collection for population into the ResourceTypesCache. @@ -734,78 +681,55 @@ public List search(String sqlSelect) throws FHIRPersistenceDataAccessE } @Override - public List searchForIds(String sqlSelect) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { - final String METHODNAME = "searchForIds"; - log.entering(CLASSNAME, METHODNAME); - - List resourceIds = new ArrayList<>(); - Connection connection = null; - PreparedStatement stmt = null; - ResultSet resultSet = null; - String errMsg; - - try { - connection = this.getConnection(); - stmt = connection.prepareStatement(sqlSelect); - resultSet = stmt.executeQuery(); - while (resultSet.next()) { - resourceIds.add(resultSet.getLong(1)); - } - } catch(FHIRPersistenceException e) { - throw e; - } catch (Throwable e) { - // Log the SQL but don't expose it in the exception - FHIRPersistenceDataAccessException fx = new FHIRPersistenceDataAccessException("Failure retrieving FHIR Resource Ids."); - errMsg = "Failure retrieving FHIR Resource Ids. SQL=" + sqlSelect; - throw severe(log, fx, errMsg, e); - } finally { - this.cleanup(resultSet, stmt, connection); - log.exiting(CLASSNAME, METHODNAME); - } - return resourceIds; - } - - @Override - public List searchByIds(List resourceIds) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { + public List searchByIds(String resourceType, List resourceIds) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { final String METHODNAME = "searchByIds"; log.entering(CLASSNAME, METHODNAME); if (resourceIds.isEmpty()) { return Collections.emptyList(); } - + Connection connection = null; PreparedStatement stmt = null; ResultSet resultSet = null; String errMsg; StringBuilder idQuery = new StringBuilder(); List resources = new ArrayList<>(); - + String stmtString = null; + long dbCallStartTime; + double dbCallDuration; + try { - idQuery.append(SQL_SEARCH_BY_IDS); + stmtString = getSearchByIdsSql(resourceType); + idQuery.append(stmtString); idQuery.append("("); + // resourceIds should have a max length of 1000 (the max page size) + StringBuilder caseStmts = new StringBuilder(); for (int i = 0; i < resourceIds.size(); i++) { if (i > 0) { idQuery.append(","); } - idQuery.append("?"); + idQuery.append(resourceIds.get(i)); + + // build up the caseStmts here so we only need to iterate the list once + caseStmts.append(WHEN + resourceIds.get(i) + THEN + i); } - idQuery.append(")"); - + idQuery.append(") " + SQL_ORDER_BY_IDS + caseStmts + END); + connection = this.getConnection(); stmt = connection.prepareStatement(idQuery.toString()); - // Inject IDs into the prepared stmt. - for (int i = 0; i < resourceIds.size(); i++) { - stmt.setObject(i+1, resourceIds.get(i)); - } + dbCallStartTime = System.nanoTime(); resultSet = stmt.executeQuery(); + dbCallDuration = (System.nanoTime()-dbCallStartTime)/1e6; + if (log.isLoggable(Level.FINE)) { + log.fine("DB search by ids complete. SQL=[" + idQuery + "] executionTime=" + dbCallDuration + "ms"); + } resources = this.createDTOs(resultSet); } catch(FHIRPersistenceException e) { throw e; } catch (Throwable e) { - // Log the SQL but don't expose it in the exception - FHIRPersistenceDataAccessException fx = new FHIRPersistenceDataAccessException("Failure retrieving FHIR Resources."); - errMsg = "Failure retrieving FHIR Resources. SQL=" + idQuery; + FHIRPersistenceDataAccessException fx = new FHIRPersistenceDataAccessException("Failure retrieving FHIR Resources"); + errMsg = "Failure retrieving FHIR Resources. SQL=[" + idQuery + "]"; throw severe(log, fx, errMsg, e); } finally { this.cleanup(resultSet, stmt, connection); @@ -814,6 +738,10 @@ public List searchByIds(List resourceIds) throws FHIRPersistence return resources; } + protected String getSearchByIdsSql(String resourceType) { + return String.format(SQL_SEARCH_BY_IDS, resourceType, resourceType); + } + @Override public int searchCount(String sqlSelectCount) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { final String METHODNAME = "searchCount"; diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/SortedQuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/SortedQuerySegmentAggregator.java index cdc03c70490..bed4442dc60 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/SortedQuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/SortedQuerySegmentAggregator.java @@ -57,15 +57,14 @@ protected SortedQuerySegmentAggregator(Class resourceType, int offset, int pa *

* A simple example query produced by this method: * 

-     * SELECT R.RESOURCE_ID,MIN(S1.STR_VALUE) FROM 
-     * Patient_RESOURCES R JOIN 
-     * Patient_LOGICAL_RESOURCES LR ON R.LOGICAL_RESOURCE_ID=LR.LOGICAL_RESOURCE_ID  JOIN 
-     * Patient_TOKEN_VALUES P1 ON P1.RESOURCE_ID=R.RESOURCE_ID  
-     * LEFT OUTER JOIN Patient_STR_VALUES S1 ON (S1.PARAMETER_NAME_ID=50 AND S1.RESOURCE_ID = R.RESOURCE_ID) WHERE 
-     * R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND 
-     * R.IS_DELETED <> 'Y' AND 
-     * P1.RESOURCE_ID = R.RESOURCE_ID AND 
-     * (P1.PARAMETER_NAME_ID=196 AND ((P1.TOKEN_VALUE = false))) 
+     * SELECT R.RESOURCE_ID,MIN(S1.STR_VALUE) FROM Patient_RESOURCES R 
+     *   JOIN Patient_LOGICAL_RESOURCES LR ON R.LOGICAL_RESOURCE_ID=LR.LOGICAL_RESOURCE_ID
+     *   JOIN Patient_TOKEN_VALUES P1 ON P1.RESOURCE_ID=R.RESOURCE_ID  
+     *   LEFT OUTER JOIN Patient_STR_VALUES S1 ON (S1.PARAMETER_NAME_ID=50 AND S1.RESOURCE_ID = R.RESOURCE_ID)
+     *   WHERE R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND 
+     *         R.IS_DELETED <> 'Y' AND 
+     *         P1.RESOURCE_ID = R.RESOURCE_ID AND 
+     *         (P1.PARAMETER_NAME_ID=196 AND ((P1.TOKEN_VALUE = false))) 
      * GROUP BY R.RESOURCE_ID  
      * ORDER BY MIN(S1.STR_VALUE) asc NULLS LAST 
      * OFFSET 0 ROWS FETCH NEXT 100 ROWS ONLY;
diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SearchPerformaceTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SearchPerformaceTest.java
index a439e316d8a..66d60723b44 100644
--- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SearchPerformaceTest.java
+++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SearchPerformaceTest.java
@@ -15,6 +15,7 @@
 import static org.testng.Assert.assertTrue;
 
 import java.lang.reflect.Method;
+import java.math.BigDecimal;
 import java.util.List;
 
 import javax.ws.rs.client.Entity;
@@ -27,9 +28,14 @@
 import com.ibm.fhir.model.resource.Bundle;
 import com.ibm.fhir.model.resource.Observation;
 import com.ibm.fhir.model.resource.Patient;
+import com.ibm.fhir.model.resource.Observation.Component;
 import com.ibm.fhir.model.test.TestUtil;
 import com.ibm.fhir.model.type.Code;
+import com.ibm.fhir.model.type.CodeableConcept;
 import com.ibm.fhir.model.type.Coding;
+import com.ibm.fhir.model.type.Decimal;
+import com.ibm.fhir.model.type.Quantity;
+import com.ibm.fhir.model.type.Uri;
 import com.ibm.fhir.model.type.code.AdministrativeGender;
 import com.ibm.fhir.model.util.FHIRUtil;
 
@@ -44,7 +50,7 @@ public class SearchPerformaceTest extends FHIRServerTestBase {
     // Controls how many observations and patients to create for the test.
     // Using invocationCount of testng can cause the testng report grows too big, so
     // this config is used to make sure all test users are created in one testng step.
-    private final int numOfPatientObservationsToCreate = 100;
+    private final int numOfPatientObservationsToCreate = 1000;
 
     /**
      * Retrieve the server's conformance statement to determine the status of certain runtime options.
@@ -87,6 +93,21 @@ public void testCreatePatientAndObservation() throws Exception {
             // create observation for the patient
             Observation observation =
                     TestUtil.buildPatientObservation(patientId, "Observation1.json");
+            observation = observation.toBuilder()
+                    .code(CodeableConcept.builder()
+                            .coding(Coding.builder()
+                                    .system(Uri.of("http://loinc.org"))
+                                    .code(Code.of("55284-4"))
+                                    .build())
+                            .build())
+                    .component(Component.builder()
+                            .code(CodeableConcept.builder().text(string("component1")).build())
+                            .value(Quantity.builder()
+                                    .value(Decimal.of(BigDecimal.valueOf(Math.random())))
+                                    .unit(string("mmHg"))
+                                    .build())
+                            .build())
+                    .build();
             Entity entity2 =
                     Entity.entity(observation, FHIRMediaType.APPLICATION_FHIR_JSON);
             response =
diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SortingTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SortingTest.java
index 34f5ec9332d..b402b3047a8 100644
--- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SortingTest.java
+++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SortingTest.java
@@ -674,6 +674,7 @@ private void assertTrueNaturalOrdering(List sortedList) {
                 }
 
                 assertTrue(prior.compareTo(current) <= 0);
+                prior = current;
             }
             done = true;
         }