From e9c0126d754df980330d95b653455a5c700d68bf Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Thu, 24 Oct 2019 22:35:44 -0400 Subject: [PATCH 1/8] issue #262 Modify SQLQueryBuild to improve search performance Signed-off-by: Albert Wang --- .../util/InclusionQuerySegmentAggregator.java | 15 +- .../jdbc/util/QuerySegmentAggregator.java | 170 ++++++++++++------ 2 files changed, 119 insertions(+), 66 deletions(-) diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java index cfc2c239ef6..9846e5bc4fa 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java @@ -196,20 +196,7 @@ private void processIncludeParameters(StringBuilder queryString, List bi queryString.append("R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND "); // ('Organization/' || LR.LOGICAL_ID IN queryString.append("('").append(includeParm.getSearchParameterTargetType()).append("/' || LR.LOGICAL_ID IN "); - // (SELECT P1.STR_VALUE FROM OBSERVATION_STR_VALUES P1 WHERE - queryString.append("(SELECT P1.STR_VALUE FROM ").append(this.resourceType.getSimpleName()).append("_STR_VALUES P1 WHERE "); - // P1.PARAMETER_NAME_ID=xx AND - queryString.append("P1.PARAMETER_NAME_ID=").append(this.getParameterNameId(includeParm.getSearchParameter())).append(" AND "); - // P1.RESOURCE_ID IN - queryString.append("P1.LOGICAL_RESOURCE_ID IN "); - // (SELECT R.LOGICAL_RESOURCE_ID - queryString.append("(SELECT R.LOGICAL_RESOURCE_ID "); - // Add FROM clause for "root" resource type - queryString.append(super.buildFromClause()); - // Add WHERE clause for "root" resource type - queryString.append(super.buildWhereClause()); - - queryString.append(")))"); + queryString.append("(?))"); this.addBindVariables(bindVariables); } diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java index 73fa5e6c183..7f9cf73464f 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java @@ -45,6 +45,7 @@ class QuerySegmentAggregator { private static final String UNION = " UNION ALL "; protected static final String ON = " ON "; private static final String JOIN = " JOIN "; + private static final String AND = " AND "; protected static final String COMBINED_RESULTS = " COMBINED_RESULTS"; private static final String DEFAULT_ORDERING = " ORDER BY R.RESOURCE_ID ASC "; @@ -242,14 +243,14 @@ protected SqlQueryData buildSystemLevelQuery(String selectRoot, String subSelect resourceTypeName = resourceIdNameMap.get(resourceTypeId) + "_"; - tempFromClause = this.buildFromClause(); + tempFromClause = this.buildFromClause2(); tempFromClause = tempFromClause.replaceAll("Resource_", resourceTypeName); if (resourceTypeProcessed) { queryString.append(UNION); } queryString.append(subSelectRoot).append(tempFromClause); resourceTypeProcessed = true; - queryString.append(this.buildWhereClause()); + queryString.append(this.buildWhereClause2()); for (SqlQueryData querySegment : this.querySegments) { allBindVariables.addAll(querySegment.getBindVariables()); } @@ -276,6 +277,107 @@ protected String buildFromClause() throws Exception { final String METHODNAME = "buildFromClause"; log.entering(CLASSNAME, METHODNAME); + StringBuilder fromClause = new StringBuilder(); + fromClause.append(MessageFormat.format(FROM_CLAUSE_ROOT, this.resourceType.getSimpleName())); + fromClause.append(" "); + + log.exiting(CLASSNAME, METHODNAME); + return fromClause.toString(); + + } + + /** + * Builds the WHERE clause for the query being generated. This method aggregates the contained query segments, and ties those segments back + * to the appropriate parameter table alias. + * @return + */ + protected String buildWhereClause() { + final String METHODNAME = "buildWhereClause"; + log.entering(CLASSNAME, METHODNAME); + boolean isLocationQuery; + + StringBuilder whereClause = new StringBuilder(); + String whereClauseSegment; + + whereClause.append(WHERE_CLAUSE_ROOT); + if (!this.querySegments.isEmpty()) { + for(int i = 0; i < this.querySegments.size(); i++) { + SqlQueryData querySegment = this.querySegments.get(i); + Parameter param = this.searchQueryParameters.get(i); + + whereClauseSegment = querySegment.getQueryString(); + if (Modifier.MISSING.equals(param.getModifier())) { + whereClause.append(AND).append(whereClauseSegment); + } else { + + whereClause.append(AND).append("R.LOGICAL_RESOURCE_ID IN (SELECT LOGICAL_RESOURCE_ID FROM "); + whereClause.append(this.resourceType.getSimpleName()); + isLocationQuery = Location.class.equals(this.resourceType) && param.getName().equals(AbstractQueryBuilder.NEAR); + switch(param.getType()) { + case URI : + case REFERENCE : + case STRING : whereClause.append("_STR_VALUES "); + break; + case NUMBER : whereClause.append("_NUMBER_VALUES "); + break; + case QUANTITY : whereClause.append("_QUANTITY_VALUES "); + break; + case DATE : whereClause.append("_DATE_VALUES "); + break; + case TOKEN : if (isLocationQuery) { + whereClause.append("_LATLNG_VALUES "); + } + else { + whereClause.append("_TOKEN_VALUES "); + } + break; + } + whereClauseSegment = whereClauseSegment.replaceAll(PARAMETER_TABLE_ALIAS + ".", ""); + whereClause.append(" WHERE ").append(whereClauseSegment).append(")"); + } + } + } + + log.exiting(CLASSNAME, METHODNAME); + return whereClause.toString(); + } + + /** + * + * @return true if this instance represents a FHIR system level search + */ + protected boolean isSystemLevelSearch() { + return Resource.class.equals(this.resourceType); + } + + /** + * Adds the appropriate pagination clauses to the passed query string buffer, based on the type + * of database we're running against. + * @param queryString A query string buffer. + * @throws Exception + */ + protected void addPaginationClauses(StringBuilder queryString) throws Exception { + + if(this.parameterDao.isDb2Database()) { + queryString.append(" LIMIT ").append(this.pageSize).append(" OFFSET ").append(this.offset); + } + else { + queryString.append(" OFFSET ").append(this.offset).append(" ROWS") + .append(" FETCH NEXT ").append(this.pageSize).append(" ROWS ONLY"); + } + } + + + /** + * Builds the FROM clause for the SQL query being generated. The appropriate Resource and Parameter table names are included + * along with an alias for each table, used by system level query. + * @return A String containing the FROM clause + * @throws Exception + */ + protected String buildFromClause2() throws Exception { + final String METHODNAME = "buildFromClause2"; + log.entering(CLASSNAME, METHODNAME); + boolean isLocationQuery; int parameterTableAliasIndex = 1; StringBuilder fromClause = new StringBuilder(); @@ -308,9 +410,16 @@ protected String buildFromClause() throws Exception { } break; } + + String resolvedTableAlias = PARAMETER_TABLE_VAR + parameterTableAliasIndex + "."; fromClause.append(PARAMETER_TABLE_VAR).append(parameterTableAliasIndex); - fromClause.append(ON).append(PARAMETER_TABLE_VAR).append(parameterTableAliasIndex).append(".LOGICAL_RESOURCE_ID=R.LOGICAL_RESOURCE_ID"); + fromClause.append(ON).append(resolvedTableAlias).append("LOGICAL_RESOURCE_ID=R.LOGICAL_RESOURCE_ID"); + SqlQueryData querySegment = this.querySegments.get(parameterTableAliasIndex-1); + String joinAndClauseSegment = querySegment.getQueryString(); + joinAndClauseSegment = joinAndClauseSegment.replaceAll(PARAMETER_TABLE_ALIAS + ".", resolvedTableAlias); + fromClause.append(AND).append(joinAndClauseSegment); + parameterTableAliasIndex++; } fromClause.append(" "); @@ -322,74 +431,31 @@ protected String buildFromClause() throws Exception { /** * Builds the WHERE clause for the query being generated. This method aggregates the contained query segments, and ties those segments back - * to the appropriate parameter table alias. + * to the appropriate parameter table alias, used by system level query. * @return */ - protected String buildWhereClause() { - final String METHODNAME = "buildWhereClause"; + protected String buildWhereClause2() { + final String METHODNAME = "buildWhereClause2"; log.entering(CLASSNAME, METHODNAME); - int parameterTableAliasIndex = 1; StringBuilder whereClause = new StringBuilder(); - String resolvedTableAlias; String whereClauseSegment; - boolean querySegmentProcessed = false; whereClause.append(WHERE_CLAUSE_ROOT); - whereClause.append(" AND "); if (!this.querySegments.isEmpty()) { for(int i = 0; i < this.querySegments.size(); i++) { SqlQueryData querySegment = this.querySegments.get(i); Parameter param = this.searchQueryParameters.get(i); - - if (querySegmentProcessed) { - whereClause.append(" AND "); - } + whereClauseSegment = querySegment.getQueryString(); - if (!Modifier.MISSING.equals(param.getModifier())) { - whereClause.append(PARAMETER_TABLE_VAR).append(parameterTableAliasIndex).append(".").append("LOGICAL_RESOURCE_ID = R.LOGICAL_RESOURCE_ID AND "); + if (Modifier.MISSING.equals(param.getModifier())) { + whereClause.append(AND).append(whereClauseSegment); } - resolvedTableAlias = PARAMETER_TABLE_VAR + parameterTableAliasIndex + "."; - whereClauseSegment = whereClauseSegment.replaceAll(PARAMETER_TABLE_ALIAS + ".", resolvedTableAlias); - whereClause.append(whereClauseSegment); - querySegmentProcessed = true; - parameterTableAliasIndex++; } } - else { - // When no query segments are present (such as in a search for all instances of a particular resource type), - // The following must be added to the WHERE clause to ensure that only the latest version of each Resource - // is retrieved. - whereClause.append("R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID"); - } log.exiting(CLASSNAME, METHODNAME); return whereClause.toString(); } - - /** - * - * @return true if this instance represents a FHIR system level search - */ - protected boolean isSystemLevelSearch() { - return Resource.class.equals(this.resourceType); - } - - /** - * Adds the appropriate pagination clauses to the passed query string buffer, based on the type - * of database we're running against. - * @param queryString A query string buffer. - * @throws Exception - */ - protected void addPaginationClauses(StringBuilder queryString) throws Exception { - - if(this.parameterDao.isDb2Database()) { - queryString.append(" LIMIT ").append(this.pageSize).append(" OFFSET ").append(this.offset); - } - else { - queryString.append(" OFFSET ").append(this.offset).append(" ROWS") - .append(" FETCH NEXT ").append(this.pageSize).append(" ROWS ONLY"); - } - } } From edb7676362acfad87e353bfba4b33d665d2aa659 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Sat, 26 Oct 2019 22:03:22 -0400 Subject: [PATCH 2/8] issue #262 unified from/where clausebuilder codes Signed-off-by: Albert Wang --- .../jdbc/util/QuerySegmentAggregator.java | 100 ++---------------- 1 file changed, 6 insertions(+), 94 deletions(-) diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java index 7f9cf73464f..fc04fdd3afb 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java @@ -243,14 +243,18 @@ protected SqlQueryData buildSystemLevelQuery(String selectRoot, String subSelect resourceTypeName = resourceIdNameMap.get(resourceTypeId) + "_"; - tempFromClause = this.buildFromClause2(); + tempFromClause = this.buildFromClause(); tempFromClause = tempFromClause.replaceAll("Resource_", resourceTypeName); if (resourceTypeProcessed) { queryString.append(UNION); } queryString.append(subSelectRoot).append(tempFromClause); resourceTypeProcessed = true; - queryString.append(this.buildWhereClause2()); + + tempFromClause = this.buildWhereClause(); + tempFromClause = tempFromClause.replaceAll("Resource_", resourceTypeName); + queryString.append(tempFromClause); + for (SqlQueryData querySegment : this.querySegments) { allBindVariables.addAll(querySegment.getBindVariables()); } @@ -366,96 +370,4 @@ protected void addPaginationClauses(StringBuilder queryString) throws Exception .append(" FETCH NEXT ").append(this.pageSize).append(" ROWS ONLY"); } } - - - /** - * Builds the FROM clause for the SQL query being generated. The appropriate Resource and Parameter table names are included - * along with an alias for each table, used by system level query. - * @return A String containing the FROM clause - * @throws Exception - */ - protected String buildFromClause2() throws Exception { - final String METHODNAME = "buildFromClause2"; - log.entering(CLASSNAME, METHODNAME); - - boolean isLocationQuery; - int parameterTableAliasIndex = 1; - StringBuilder fromClause = new StringBuilder(); - String resourceTypeName = this.resourceType.getSimpleName(); - fromClause.append(MessageFormat.format(FROM_CLAUSE_ROOT, this.resourceType.getSimpleName())); - - for (Parameter searchQueryParm : this.searchQueryParameters) { - if (Modifier.MISSING.equals(searchQueryParm.getModifier())) { - // No need to join on the VALUES table for search params with the :missing modifier - continue; - } - fromClause.append(JOIN).append(resourceTypeName); - isLocationQuery = Location.class.equals(this.resourceType) && searchQueryParm.getName().equals(AbstractQueryBuilder.NEAR); - switch(searchQueryParm.getType()) { - case URI : - case REFERENCE : - case STRING : fromClause.append("_STR_VALUES "); - break; - case NUMBER : fromClause.append("_NUMBER_VALUES "); - break; - case QUANTITY : fromClause.append("_QUANTITY_VALUES "); - break; - case DATE : fromClause.append("_DATE_VALUES "); - break; - case TOKEN : if (isLocationQuery) { - fromClause.append("_LATLNG_VALUES "); - } - else { - fromClause.append("_TOKEN_VALUES "); - } - break; - } - - String resolvedTableAlias = PARAMETER_TABLE_VAR + parameterTableAliasIndex + "."; - fromClause.append(PARAMETER_TABLE_VAR).append(parameterTableAliasIndex); - fromClause.append(ON).append(resolvedTableAlias).append("LOGICAL_RESOURCE_ID=R.LOGICAL_RESOURCE_ID"); - - SqlQueryData querySegment = this.querySegments.get(parameterTableAliasIndex-1); - String joinAndClauseSegment = querySegment.getQueryString(); - joinAndClauseSegment = joinAndClauseSegment.replaceAll(PARAMETER_TABLE_ALIAS + ".", resolvedTableAlias); - fromClause.append(AND).append(joinAndClauseSegment); - - parameterTableAliasIndex++; - } - fromClause.append(" "); - - log.exiting(CLASSNAME, METHODNAME); - return fromClause.toString(); - - } - - /** - * Builds the WHERE clause for the query being generated. This method aggregates the contained query segments, and ties those segments back - * to the appropriate parameter table alias, used by system level query. - * @return - */ - protected String buildWhereClause2() { - final String METHODNAME = "buildWhereClause2"; - log.entering(CLASSNAME, METHODNAME); - - StringBuilder whereClause = new StringBuilder(); - String whereClauseSegment; - - whereClause.append(WHERE_CLAUSE_ROOT); - if (!this.querySegments.isEmpty()) { - for(int i = 0; i < this.querySegments.size(); i++) { - SqlQueryData querySegment = this.querySegments.get(i); - Parameter param = this.searchQueryParameters.get(i); - - whereClauseSegment = querySegment.getQueryString(); - if (Modifier.MISSING.equals(param.getModifier())) { - whereClause.append(AND).append(whereClauseSegment); - } - } - } - - log.exiting(CLASSNAME, METHODNAME); - return whereClause.toString(); - } - } From 854ec5be6f513fb34b692df3f51d6ee9e73b490d Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Mon, 28 Oct 2019 16:26:43 -0400 Subject: [PATCH 3/8] issue #262 Performance enhancement for include search Signed-off-by: Albert Wang --- .../persistence/jdbc/dao/api/ResourceDAO.java | 12 +++- .../jdbc/dao/impl/FHIRDbDAOImpl.java | 55 +++++++++++++++++++ .../jdbc/dao/impl/ResourceDAOImpl.java | 15 +++++ .../util/InclusionQuerySegmentAggregator.java | 39 +++++++++++-- .../jdbc/util/QuerySegmentAggregator.java | 39 ------------- 5 files changed, 116 insertions(+), 44 deletions(-) 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 b33a9c5d8dc..285d9bd8474 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 @@ -75,7 +75,7 @@ List history(String resourceType, String logicalId, Timestamp fromDate */ int historyCount(String resourceType, String logicalId, Timestamp fromDateTime) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; - + /** * Executes the search contained in the passed SqlQueryData, using it's encapsulated search string and bind variables. * @param queryData - Contains a search string and (optionally) bind variables. @@ -84,6 +84,16 @@ int historyCount(String resourceType, String logicalId, Timestamp fromDateTime) * @throws FHIRPersistenceDBConnectException */ List search(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; + + /** + * Executes the search contained in the passed SqlQueryData, using it's encapsulated search string and bind variables. + * @param queryData - Contains a search string and (optionally) bind variables. + * @return List A list of strings satisfying the passed search. + * @throws FHIRPersistenceDataAccessException + * @throws FHIRPersistenceDBConnectException + */ + List searchSTR_VALUES(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; + /** * Executes the passed fully-formed SQL Select statement and returns the results diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/FHIRDbDAOImpl.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/FHIRDbDAOImpl.java index 2d13740f2a7..ba8b2b449b5 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/FHIRDbDAOImpl.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/FHIRDbDAOImpl.java @@ -525,4 +525,59 @@ protected FHIRPersistenceDataAccessException buildExceptionWithIssue(String msg, Issue ooi = FHIRUtil.buildOperationOutcomeIssue(msg, issueType); return new FHIRPersistenceDataAccessException(msg).withIssue(ooi); } + + /** + * Creates and executes a PreparedStatement using the passed parameters that returns a collection of String values. + * @param sql - The SQL template to execute. + * @param searchArgs - An array of arguments to be substituted into the SQL template. + * @return List - A List of strings resulting from the executed query. + * @throws FHIRPersistenceDataAccessException + * @throws FHIRPersistenceDBConnectException + */ + protected List runQuery_STR_VALUES(String sql, Object... searchArgs) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { + final String METHODNAME = "runQuery_STR_VALUES"; + log.entering(CLASSNAME, METHODNAME); + List strValues = new ArrayList(); + Connection connection = null; + PreparedStatement stmt = null; + ResultSet resultSet = null; + String errMsg; + long dbCallStartTime; + double dbCallDuration; + + try { + connection = this.getConnection(); + stmt = connection.prepareStatement(sql); + // Inject arguments into the prepared stmt. + for (int i = 0; i bi // R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND queryString.append("R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND "); // ('Organization/' || LR.LOGICAL_ID IN - queryString.append("('").append(includeParm.getSearchParameterTargetType()).append("/' || LR.LOGICAL_ID IN "); - queryString.append("(?))"); - - this.addBindVariables(bindVariables); + queryString.append("('").append(includeParm.getSearchParameterTargetType()).append("/' || LR.LOGICAL_ID IN ("); + + // Run the following sub Query only once to improve performance. + StringBuilder subQueryString = new StringBuilder(); + // SELECT P1.STR_VALUE FROM OBSERVATION_STR_VALUES P1 WHERE + subQueryString.append("SELECT P1.STR_VALUE FROM ").append(this.resourceType.getSimpleName()).append("_STR_VALUES P1 WHERE "); + // P1.PARAMETER_NAME_ID=xx AND + subQueryString.append("P1.PARAMETER_NAME_ID=").append(this.getParameterNameId(includeParm.getSearchParameter())).append(" AND "); + // P1.RESOURCE_ID IN + subQueryString.append("P1.LOGICAL_RESOURCE_ID IN "); + // (SELECT R.LOGICAL_RESOURCE_ID + subQueryString.append("(SELECT R.LOGICAL_RESOURCE_ID "); + // Add FROM clause for "root" resource type + subQueryString.append(super.buildFromClause()); + // Add WHERE clause for "root" resource type + subQueryString.append(super.buildWhereClause()); + subQueryString.append(")"); + + // ('Patient/326faf57-2aff-4ae5-ab41-87eace6c8f33') + SqlQueryData subQueryData = new SqlQueryData(subQueryString.toString(), bindVariables); + boolean isFirstItem = true; + for (String strValue: this.resourceDao.searchSTR_VALUES(subQueryData)) { + if (!isFirstItem) { + queryString.append(" , "); + } + if (strValue != null) { + queryString.append("'").append(strValue).append("'"); + isFirstItem = false; + } + } + // if nothing added so far, then need to add '', otherwise sql will fail. + if (isFirstItem) { + queryString.append("''"); + } + queryString.append("))"); } log.exiting(CLASSNAME, METHODNAME); } diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java index fc04fdd3afb..c3c50eaf85e 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/QuerySegmentAggregator.java @@ -39,12 +39,10 @@ class QuerySegmentAggregator { private static final String SYSTEM_LEVEL_SUBSELECT_COUNT_ROOT = " SELECT R.RESOURCE_ID "; protected static final String FROM_CLAUSE_ROOT = "FROM {0}_RESOURCES R JOIN {0}_LOGICAL_RESOURCES LR ON R.LOGICAL_RESOURCE_ID=LR.LOGICAL_RESOURCE_ID AND R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID "; protected static final String WHERE_CLAUSE_ROOT = "WHERE R.IS_DELETED <> 'Y'"; - private static final String PARAMETER_TABLE_VAR = "P"; protected static final String PARAMETER_TABLE_ALIAS = "pX"; private static final String FROM = " FROM "; private static final String UNION = " UNION ALL "; protected static final String ON = " ON "; - private static final String JOIN = " JOIN "; private static final String AND = " AND "; protected static final String COMBINED_RESULTS = " COMBINED_RESULTS"; private static final String DEFAULT_ORDERING = " ORDER BY R.RESOURCE_ID ASC "; @@ -102,16 +100,6 @@ protected void addQueryData(SqlQueryData querySegment,Parameter queryParm) { /** * Builds a complete SQL Query based upon the encapsulated query segments and bind variables. - * A simple example query produced by this method: - * - * SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID FROM - * PATIENT_RESOURCES R, PATIENT_LOGICAL_RESOURCES LR, PATIENT_STR_VALUES P1 WHERE - * R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND - * P1.LOGICAL_RESOURCE_ID = LR.LOGICAL_RESOURCE_ID AND - * (P1.PARAMETER_NAME_ID = 4 AND - * P1.STR_VALUE LIKE ? ESCAPE '+') - * ORDER BY r.RESOURCE_ID ASC OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY - * * @return SqlQueryData - contains the complete SQL query string and any associated bind variables. * @throws Exception */ @@ -148,15 +136,6 @@ protected SqlQueryData buildQuery() throws Exception { /** * Builds a complete SQL count query based upon the encapsulated query segments and bind variables. - * A simple example query produced by this method: - * - * SELECT COUNT(R.RESOURCE_ID)FROM - * PATIENT_RESOURCES R, PATIENT_LOGICAL_RESOURCES LR, PATIENT_STR_VALUES P1 WHERE - * R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND - * (P1.LOGICAL_RESOURCE_ID = R.LOGICAL_RESOURCE_ID AND - * (P1.PARAMETER_NAME_ID = 4 AND - * P1.STR_VALUE LIKE ? ESCAPE '+')) - * * @return SqlQueryData - contains the complete SQL count query string and any associated bind variables. * @throws Exception */ @@ -193,24 +172,6 @@ protected SqlQueryData buildCountQuery() throws Exception { * Build a system level query or count query, based upon the encapsulated query segments and bind variables and * the passed select-root strings. * A FHIR system level query spans multiple resource types, and therefore spans multiple tables in the database. - * Here is an example of a system level query, assuming that only 3 different resource types have been persisted - * in the database: - * SELECT RESOURCE_ID, LOGICAL_RESOURCE_ID, VERSION_ID, LAST_UPDATED, IS_DELETED, DATA, LOGICAL_ID FROM - * (SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID FROM - * RiskAssessment_RESOURCES R, RiskAssessment_LOGICAL_RESOURCES LR , RiskAssessment_DATE_VALUES P1 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=3 AND ((P1.DATE_VALUE = '2017-06-15 21:30:58.251'))) - * UNION ALL - * SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID FROM - * Group_RESOURCES R, Group_LOGICAL_RESOURCES LR , Group_DATE_VALUES P1 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=3 AND ((P1.DATE_VALUE = '2017-06-15 21:30:58.251'))) - * UNION ALL - * SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID FROM - * Questionnaire_RESOURCES R, Questionnaire_LOGICAL_RESOURCES LR , Questionnaire_DATE_VALUES P1 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=3 AND ((P1.DATE_VALUE = '2017-06-15 21:30:58.251')))) COMBINED_RESULTS; - * * @param selectRoot - The text of the outer SELECT ('SELECT' to 'FROM') * @param subSelectRoot - The text of the inner SELECT root to use in each sub-select * @param addFinalClauses - Indicates whether or not ordering and pagination clauses should be generated. From 32ed7ce8c26ca728c470116a4ff0e0cbb7096843 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Mon, 28 Oct 2019 18:40:10 -0400 Subject: [PATCH 4/8] issue #262 rename the new function in ResourceDAO interface Signed-off-by: Albert Wang --- .../com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java | 3 ++- .../ibm/fhir/persistence/jdbc/dao/impl/ResourceDAOImpl.java | 2 +- .../persistence/jdbc/util/InclusionQuerySegmentAggregator.java | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) 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 285d9bd8474..1829d0ee25c 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 @@ -87,12 +87,13 @@ int historyCount(String resourceType, String logicalId, Timestamp fromDateTime) /** * Executes the search contained in the passed SqlQueryData, using it's encapsulated search string and bind variables. + * This function is used by _include search only * @param queryData - Contains a search string and (optionally) bind variables. * @return List A list of strings satisfying the passed search. * @throws FHIRPersistenceDataAccessException * @throws FHIRPersistenceDBConnectException */ - List searchSTR_VALUES(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; + List searchStringValues(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; /** 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 abb708c6aea..d37625edd2e 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 @@ -873,7 +873,7 @@ public int searchCount(String sqlSelectCount) throws FHIRPersistenceDataAccessEx } @Override - public List searchSTR_VALUES(SqlQueryData queryData) + public List searchStringValues(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException { final String METHODNAME = "searchSTR_VALUES"; log.entering(CLASSNAME, METHODNAME); diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java index 1e472c5c020..24c44ffb96d 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java @@ -220,7 +220,7 @@ private void processIncludeParameters(StringBuilder queryString, List bi // ('Patient/326faf57-2aff-4ae5-ab41-87eace6c8f33') SqlQueryData subQueryData = new SqlQueryData(subQueryString.toString(), bindVariables); boolean isFirstItem = true; - for (String strValue: this.resourceDao.searchSTR_VALUES(subQueryData)) { + for (String strValue: this.resourceDao.searchStringValues(subQueryData)) { if (!isFirstItem) { queryString.append(" , "); } From e3196f81445de9f6e4e4e8590d5dead25bbaeda5 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Tue, 29 Oct 2019 07:28:34 -0400 Subject: [PATCH 5/8] issue #262 update comments per Lee's suggestion Signed-off-by: Albert Wang --- .../com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java | 3 +++ 1 file changed, 3 insertions(+) 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 1829d0ee25c..c78cbe0a5eb 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 @@ -92,6 +92,9 @@ int historyCount(String resourceType, String logicalId, Timestamp fromDateTime) * @return List A list of strings satisfying the passed search. * @throws FHIRPersistenceDataAccessException * @throws FHIRPersistenceDBConnectException + * @implNote This method is used within searches which have _include or _revinclude parameters + * in order to return a list of Reference values (e.g. {@code "Patient/"}) + * to use for filtering the list of resources to be included with the response. */ List searchStringValues(SqlQueryData queryData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException; From 1ea762b5f577538d1ab2e4ff817de7bac82fa85d Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Tue, 29 Oct 2019 08:09:16 -0400 Subject: [PATCH 6/8] issue #262 change comments Signed-off-by: Albert Wang --- .../java/com/ibm/fhir/persistence/jdbc/dao/api/ResourceDAO.java | 1 - 1 file changed, 1 deletion(-) 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 c78cbe0a5eb..1eba8cd9a23 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 @@ -87,7 +87,6 @@ int historyCount(String resourceType, String logicalId, Timestamp fromDateTime) /** * Executes the search contained in the passed SqlQueryData, using it's encapsulated search string and bind variables. - * This function is used by _include search only * @param queryData - Contains a search string and (optionally) bind variables. * @return List A list of strings satisfying the passed search. * @throws FHIRPersistenceDataAccessException From 5e5f35516c12f936b67ab4283bb4a68c0da96811 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Tue, 29 Oct 2019 09:06:35 -0400 Subject: [PATCH 7/8] issue #262 update comments and split a new function Signed-off-by: Albert Wang --- .../util/InclusionQuerySegmentAggregator.java | 77 +++++++++++-------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java index 24c44ffb96d..07da0eb0ad8 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java @@ -183,6 +183,47 @@ protected SqlQueryData buildQuery() throws Exception { return queryData; } + /** + * Appends values like ({@code ('Patient/', 'Patient/' ...)}) to the queryString + */ + private void executeIncludeSubQuery(StringBuilder queryString, InclusionParameter includeParm, + List bindVariables) throws Exception{ + StringBuilder subQueryString = new StringBuilder(); + // SELECT P1.STR_VALUE FROM OBSERVATION_STR_VALUES P1 WHERE + subQueryString.append("SELECT P1.STR_VALUE FROM ").append(this.resourceType.getSimpleName()).append("_STR_VALUES P1 WHERE "); + // P1.PARAMETER_NAME_ID=xx AND + subQueryString.append("P1.PARAMETER_NAME_ID=").append(this.getParameterNameId(includeParm.getSearchParameter())).append(" AND "); + // P1.RESOURCE_ID IN + subQueryString.append("P1.LOGICAL_RESOURCE_ID IN "); + // (SELECT R.LOGICAL_RESOURCE_ID + subQueryString.append("(SELECT R.LOGICAL_RESOURCE_ID "); + // Add FROM clause for "root" resource type + subQueryString.append(super.buildFromClause()); + // Add WHERE clause for "root" resource type + subQueryString.append(super.buildWhereClause()); + subQueryString.append(")"); + + //The subquery should return a list of strings in the FHIR Reference String value format + //(e.g. {@code "Patient/"}) + SqlQueryData subQueryData = new SqlQueryData(subQueryString.toString(), bindVariables); + boolean isFirstItem = true; + for (String strValue: this.resourceDao.searchStringValues(subQueryData)) { + if (!isFirstItem) { + queryString.append(" , "); + } + if (strValue != null) { + queryString.append("'").append(strValue).append("'"); + isFirstItem = false; + } + } + // if nothing added so far, then need to add '', otherwise sql will fail. + if (isFirstItem) { + queryString.append("''"); + } + queryString.append("))"); + } + + private void processIncludeParameters(StringBuilder queryString, List bindVariables) throws Exception { final String METHODNAME = "processIncludeParameters"; log.entering(CLASSNAME, METHODNAME); @@ -201,39 +242,9 @@ private void processIncludeParameters(StringBuilder queryString, List bi // ('Organization/' || LR.LOGICAL_ID IN queryString.append("('").append(includeParm.getSearchParameterTargetType()).append("/' || LR.LOGICAL_ID IN ("); - // Run the following sub Query only once to improve performance. - StringBuilder subQueryString = new StringBuilder(); - // SELECT P1.STR_VALUE FROM OBSERVATION_STR_VALUES P1 WHERE - subQueryString.append("SELECT P1.STR_VALUE FROM ").append(this.resourceType.getSimpleName()).append("_STR_VALUES P1 WHERE "); - // P1.PARAMETER_NAME_ID=xx AND - subQueryString.append("P1.PARAMETER_NAME_ID=").append(this.getParameterNameId(includeParm.getSearchParameter())).append(" AND "); - // P1.RESOURCE_ID IN - subQueryString.append("P1.LOGICAL_RESOURCE_ID IN "); - // (SELECT R.LOGICAL_RESOURCE_ID - subQueryString.append("(SELECT R.LOGICAL_RESOURCE_ID "); - // Add FROM clause for "root" resource type - subQueryString.append(super.buildFromClause()); - // Add WHERE clause for "root" resource type - subQueryString.append(super.buildWhereClause()); - subQueryString.append(")"); - - // ('Patient/326faf57-2aff-4ae5-ab41-87eace6c8f33') - SqlQueryData subQueryData = new SqlQueryData(subQueryString.toString(), bindVariables); - boolean isFirstItem = true; - for (String strValue: this.resourceDao.searchStringValues(subQueryData)) { - if (!isFirstItem) { - queryString.append(" , "); - } - if (strValue != null) { - queryString.append("'").append(strValue).append("'"); - isFirstItem = false; - } - } - // if nothing added so far, then need to add '', otherwise sql will fail. - if (isFirstItem) { - queryString.append("''"); - } - queryString.append("))"); + // Execute sub query to get the string values for constructing the query string. + // This avoids DB engine to run this sub query once for each record in the previously joined tables. + executeIncludeSubQuery(queryString, includeParm, bindVariables); } log.exiting(CLASSNAME, METHODNAME); } From 4c3614e451488bf5ad03d9bd784fa599d38cd30f Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Tue, 29 Oct 2019 09:29:13 -0400 Subject: [PATCH 8/8] issue #262 minor change Signed-off-by: Albert Wang --- .../jdbc/util/InclusionQuerySegmentAggregator.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java index 07da0eb0ad8..f1018576981 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/InclusionQuerySegmentAggregator.java @@ -201,8 +201,9 @@ private void executeIncludeSubQuery(StringBuilder queryString, InclusionParamete subQueryString.append(super.buildFromClause()); // Add WHERE clause for "root" resource type subQueryString.append(super.buildWhereClause()); - subQueryString.append(")"); + subQueryString.append(")"); + queryString.append("("); //The subquery should return a list of strings in the FHIR Reference String value format //(e.g. {@code "Patient/"}) SqlQueryData subQueryData = new SqlQueryData(subQueryString.toString(), bindVariables); @@ -220,14 +221,14 @@ private void executeIncludeSubQuery(StringBuilder queryString, InclusionParamete if (isFirstItem) { queryString.append("''"); } - queryString.append("))"); + queryString.append(")"); } private void processIncludeParameters(StringBuilder queryString, List bindVariables) throws Exception { final String METHODNAME = "processIncludeParameters"; log.entering(CLASSNAME, METHODNAME); - + for (InclusionParameter includeParm : this.includeParameters) { // UNION ALL queryString.append(UNION_ALL); @@ -240,11 +241,12 @@ private void processIncludeParameters(StringBuilder queryString, List bi // R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND queryString.append("R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND "); // ('Organization/' || LR.LOGICAL_ID IN - queryString.append("('").append(includeParm.getSearchParameterTargetType()).append("/' || LR.LOGICAL_ID IN ("); + queryString.append("('").append(includeParm.getSearchParameterTargetType()).append("/' || LR.LOGICAL_ID IN "); // Execute sub query to get the string values for constructing the query string. // This avoids DB engine to run this sub query once for each record in the previously joined tables. executeIncludeSubQuery(queryString, includeParm, bindVariables); + queryString.append(")"); } log.exiting(CLASSNAME, METHODNAME); }