honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid duplicate predicate in _id queries (#1761) 

* Avoid duplicate predicate in _id queries

* Add changelog

* Change to trigger CI

* Address review comments
This commit is contained in:
James Agnew 2020-03-17 11:12:58 -04:00 committed by GitHub
parent 6b019492fe
commit 5867d62d62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 103 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1761-avoid-duplicate_restype.yaml Normal file
View File

@ -0,0 +1,7 @@
---
type: fix
issue: 1761
title: "A minor regression in 4.2.0 was introduced, where JPA searches using the `_id` search parameter often had
a duplicate SQL predicate in their where clause. This issue may not have caused any bad effects on some environments,
but it did look strange in SQL logs and has been corrected."

44
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java
View File

@ -30,18 +30,30 @@ import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.dao.data.IResourceSearchViewDao;
import ca.uhn.fhir.jpa.dao.data.IResourceTagDao;
import ca.uhn.fhir.jpa.dao.index.IdHelperService;
import ca.uhn.fhir.jpa.dao.predicate.*;
import ca.uhn.fhir.jpa.dao.predicate.PredicateBuilder;
import ca.uhn.fhir.jpa.dao.predicate.PredicateBuilderFactory;
import ca.uhn.fhir.jpa.dao.predicate.QueryRoot;
import ca.uhn.fhir.jpa.dao.predicate.SearchBuilderJoinEnum;
import ca.uhn.fhir.jpa.dao.predicate.SearchBuilderJoinKey;
import ca.uhn.fhir.jpa.entity.ResourceSearchView;
import ca.uhn.fhir.jpa.interceptor.JpaPreResourceAccessDetails;
import ca.uhn.fhir.jpa.model.cross.ResourcePersistentId;
import ca.uhn.fhir.jpa.model.entity.*;
import ca.uhn.fhir.jpa.model.entity.BaseResourceIndexedSearchParam;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedCompositeStringUnique;
import ca.uhn.fhir.jpa.model.entity.ResourceLink;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.model.entity.ResourceTag;
import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails;
import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage;
import ca.uhn.fhir.jpa.searchparam.JpaRuntimeSearchParam;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry;
import ca.uhn.fhir.jpa.searchparam.util.Dstu3DistanceHelper;
import ca.uhn.fhir.jpa.util.*;
import ca.uhn.fhir.jpa.util.BaseIterator;
import ca.uhn.fhir.jpa.util.CurrentThreadCaptureQueriesListener;
import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster;
import ca.uhn.fhir.jpa.util.ScrollableResultsIterator;
import ca.uhn.fhir.jpa.util.SqlQueryList;
import ca.uhn.fhir.model.api.IQueryParameterType;
import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.api.Include;
@ -81,10 +93,27 @@ import javax.persistence.EntityManager;
import javax.persistence.PersistenceContext;
import javax.persistence.PersistenceContextType;
import javax.persistence.TypedQuery;
import javax.persistence.criteria.*;
import java.util.*;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.From;
import javax.persistence.criteria.Join;
import javax.persistence.criteria.JoinType;
import javax.persistence.criteria.Order;
import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.apache.commons.lang3.StringUtils.*;
import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
/**
* The SearchBuilder is responsible for actually forming the SQL query that handles
@ -316,7 +345,8 @@ public class SearchBuilder implements ISearchBuilder {
* If we have any joins to index tables, we get this behaviour already guaranteed so we don't
* need an explicit predicate for it.
*/
if (!myQueryRoot.hasIndexJoins()) {
boolean haveNoIndexSearchParams = myParams.size() == 0 || myParams.keySet().stream().allMatch(t -> t.startsWith("_"));
if (haveNoIndexSearchParams) {
if (myParams.getEverythingMode() == null) {
myQueryRoot.addPredicate(myBuilder.equal(myQueryRoot.get("myResourceType"), myResourceName));
}

2
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderResourceId.java
View File

@ -121,12 +121,10 @@ class PredicateBuilderResourceId extends BasePredicateBuilder {
default:
case eq:
codePredicates.add(theRoot.get("myId").as(Long.class).in(ResourcePersistentId.toLongList(allOrPids)));
codePredicates.add(myBuilder.equal(myQueryRoot.get("myResourceType"), theResourceName));
nextPredicate = myBuilder.and(toArray(codePredicates));
break;
case ne:
codePredicates.add(theRoot.get("myId").as(Long.class).in(ResourcePersistentId.toLongList(allOrPids)).not());
codePredicates.add(myBuilder.equal(myQueryRoot.get("myResourceType"), theResourceName));
nextPredicate = myBuilder.and(toArray(codePredicates));
break;
}

59
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java
View File

@ -7,6 +7,7 @@ import ca.uhn.fhir.jpa.model.entity.*;
import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap.EverythingModeEnum;
import ca.uhn.fhir.jpa.util.SqlQuery;
import ca.uhn.fhir.jpa.util.TestUtil;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.model.api.TemporalPrecisionEnum;
@ -1076,6 +1077,64 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
}
@Test
public void testSearchByIdParam_QueryIsMinimal() {
// With only an _id parameter
{
SearchParameterMap params = new SearchParameterMap();
params.setLoadSynchronous(true);
params.add("_id", new StringParam("DiagnosticReport/123"));
myCaptureQueriesListener.clear();
myDiagnosticReportDao.search(params).size();
List<SqlQuery> selectQueries = myCaptureQueriesListener.getSelectQueriesForCurrentThread();
assertEquals(1, selectQueries.size());
String sqlQuery = selectQueries.get(0).getSql(true, true).toLowerCase();
ourLog.debug("SQL Query:\n{}", sqlQuery);
assertEquals(1, StringUtils.countMatches(sqlQuery, "resourceta0_.res_id in"));
assertEquals(0, StringUtils.countMatches(sqlQuery, "join"));
assertEquals(1, StringUtils.countMatches(sqlQuery, "resourceta0_.res_type='diagnosticreport'"));
assertEquals(1, StringUtils.countMatches(sqlQuery, "resourceta0_.res_deleted_at is null"));
}
// With an _id parameter and a standard search param
{
SearchParameterMap params = new SearchParameterMap();
params.setLoadSynchronous(true);
params.add("_id", new StringParam("DiagnosticReport/123"));
params.add("code", new TokenParam("foo", "bar"));
myCaptureQueriesListener.clear();
myDiagnosticReportDao.search(params).size();
List<SqlQuery> selectQueries = myCaptureQueriesListener.getSelectQueriesForCurrentThread();
assertEquals(1, selectQueries.size());
String sqlQuery = selectQueries.get(0).getSql(true, true).toLowerCase();
ourLog.info("SQL Query:\n{}", sqlQuery);
assertEquals(1, StringUtils.countMatches(sqlQuery, "resourceta0_.res_id in"));
assertEquals(1, StringUtils.countMatches(sqlQuery, "join"));
assertEquals(1, StringUtils.countMatches(sqlQuery, "hash_sys_and_value"));
assertEquals(0, StringUtils.countMatches(sqlQuery, "diagnosticreport"));
assertEquals(0, StringUtils.countMatches(sqlQuery, "res_deleted_at"));
}
}
@Test
public void testSearchByIdParamAndOtherSearchParam_QueryIsMinimal() {
SearchParameterMap params = new SearchParameterMap();
params.add("_id", new StringParam("DiagnosticReport/123"));
params.add("_id", new StringParam("DiagnosticReport/123"));
myCaptureQueriesListener.clear();
myDiagnosticReportDao.search(params).size();
List<SqlQuery> selectQueries = myCaptureQueriesListener.getSelectQueries();
assertEquals(1, selectQueries.size());
String sqlQuery = selectQueries.get(0).getSql(true, true).toLowerCase();
ourLog.info("SQL Query:\n{}", sqlQuery);
assertEquals(1, StringUtils.countMatches(sqlQuery, "resourceta0_.res_id in"));
assertEquals(0, StringUtils.countMatches(sqlQuery, "join"));
assertEquals(1, StringUtils.countMatches(sqlQuery,"resourceta0_.res_type='diagnosticreport'"));
assertEquals(1, StringUtils.countMatches(sqlQuery,"resourceta0_.res_deleted_at is null"));
}
@Test
public void testSearchByIdParamAnd() {
IIdType id1;