From 5d540d9208ea2f1253a6edd0c17ec2887c72165b Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sun, 27 Jan 2019 20:03:48 -0500 Subject: [PATCH 1/2] Query optimization in JPA --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 148 ++++++++++-------- .../jpa/config/CaptureQueriesListener.java | 92 +++++++++++ .../ca/uhn/fhir/jpa/config/TestR4Config.java | 1 + .../java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java | 2 + .../r4/FhirResourceDaoR4SearchNoFtTest.java | 48 ++++++ pom.xml | 2 +- src/changes/changes.xml | 6 + 7 files changed, 234 insertions(+), 65 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index e5c0a909037..a5187798ee0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -80,7 +80,6 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Scope; import org.springframework.stereotype.Component; -import org.thymeleaf.util.ListUtils; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -385,7 +384,8 @@ public class SearchBuilder implements ISearchBuilder { List codePredicates = new ArrayList<>(); - for (IQueryParameterType nextOr : theList) { + for (int orIdx = 0; orIdx < theList.size(); orIdx++) { + IQueryParameterType nextOr = theList.get(orIdx); if (nextOr instanceof ReferenceParam) { ReferenceParam ref = (ReferenceParam) nextOr; @@ -496,15 +496,16 @@ public class SearchBuilder implements ISearchBuilder { boolean foundChainMatch = false; - String chain = ref.getChain(); - String remainingChain = null; - int chainDotIndex = chain.indexOf('.'); - if (chainDotIndex != -1) { - remainingChain = chain.substring(chainDotIndex + 1); - chain = chain.substring(0, chainDotIndex); - } - for (Class nextType : resourceTypes) { + + String chain = ref.getChain(); + String remainingChain = null; + int chainDotIndex = chain.indexOf('.'); + if (chainDotIndex != -1) { + remainingChain = chain.substring(chainDotIndex + 1); + chain = chain.substring(0, chainDotIndex); + } + RuntimeResourceDefinition typeDef = myContext.getResourceDefinition(nextType); String subResourceName = typeDef.getName(); @@ -531,37 +532,29 @@ public class SearchBuilder implements ISearchBuilder { } } - IQueryParameterType chainValue; - if (remainingChain != null) { - if (param == null || param.getParamType() != RestSearchParameterTypeEnum.REFERENCE) { - ourLog.debug("Type {} parameter {} is not a reference, can not chain {}", nextType.getSimpleName(), chain, remainingChain); + ArrayList orValues = Lists.newArrayList(); + + for (IQueryParameterType next : theList) { + String nextValue = next.getValueAsQueryToken(myContext); + IQueryParameterType chainValue = mapReferenceChainToRawParamType(remainingChain, param, theParamName, qualifier, nextType, chain, isMeta, nextValue); + if (chainValue == null) { continue; } - - chainValue = new ReferenceParam(); - chainValue.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId); - ((ReferenceParam) chainValue).setChain(remainingChain); - } else if (isMeta) { - IQueryParameterType type = myMatchUrlService.newInstanceType(chain); - type.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId); - chainValue = type; - } else { - chainValue = toParameterType(param, qualifier, resourceId); + foundChainMatch = true; + orValues.add(chainValue); } - foundChainMatch = true; - Subquery subQ = myResourceTableQuery.subquery(Long.class); Root subQfrom = subQ.from(ResourceTable.class); subQ.select(subQfrom.get("myId").as(Long.class)); List> andOrParams = new ArrayList<>(); - andOrParams.add(Collections.singletonList(chainValue)); + andOrParams.add(orValues); /* * We're doing a chain call, so push the current query root * and predicate list down and put new ones at the top of the - * stack and run a subuery + * stack and run a subquery */ Root stackRoot = myResourceTableRoot; ArrayList stackPredicates = myPredicates; @@ -573,9 +566,11 @@ public class SearchBuilder implements ISearchBuilder { // Create the subquery predicates myPredicates.add(myBuilder.equal(myResourceTableRoot.get("myResourceType"), subResourceName)); myPredicates.add(myBuilder.isNull(myResourceTableRoot.get("myDeleted"))); - searchForIdsWithAndOr(subResourceName, chain, andOrParams); - subQ.where(toArray(myPredicates)); + if (foundChainMatch) { + searchForIdsWithAndOr(subResourceName, chain, andOrParams); + subQ.where(toArray(myPredicates)); + } /* * Pop the old query root and predicate list back @@ -593,6 +588,10 @@ public class SearchBuilder implements ISearchBuilder { if (!foundChainMatch) { throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "invalidParameterChain", theParamName + '.' + ref.getChain())); } + + myPredicates.add(myBuilder.or(toArray(codePredicates))); + return; + } } else { @@ -604,6 +603,28 @@ public class SearchBuilder implements ISearchBuilder { myPredicates.add(myBuilder.or(toArray(codePredicates))); } + private IQueryParameterType mapReferenceChainToRawParamType(String remainingChain, RuntimeSearchParam param, String theParamName, String qualifier, Class nextType, String chain, boolean isMeta, String resourceId) { + IQueryParameterType chainValue; + if (remainingChain != null) { + if (param == null || param.getParamType() != RestSearchParameterTypeEnum.REFERENCE) { + ourLog.debug("Type {} parameter {} is not a reference, can not chain {}", nextType.getSimpleName(), chain, remainingChain); + return null; + } + + chainValue = new ReferenceParam(); + chainValue.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId); + ((ReferenceParam) chainValue).setChain(remainingChain); + } else if (isMeta) { + IQueryParameterType type = myMatchUrlService.newInstanceType(chain); + type.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId); + chainValue = type; + } else { + chainValue = toParameterType(param, qualifier, resourceId); + } + + return chainValue; + } + private void addPredicateResourceId(List> theValues) { for (List nextValue : theValues) { Set orPids = new HashSet<>(); @@ -794,24 +815,27 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateToken(String theResourceName, String theParamName, List theList) { - Join join = createOrReuseJoin(JoinEnum.TOKEN, theParamName); - if (theList.get(0).getMissing() != null) { + Join join = createOrReuseJoin(JoinEnum.TOKEN, theParamName); addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } List codePredicates = new ArrayList<>(); + Join join = null; for (IQueryParameterType nextOr : theList) { if (nextOr instanceof TokenParam) { TokenParam id = (TokenParam) nextOr; if (id.isText()) { addPredicateString(theResourceName, theParamName, theList); - continue; + break; } } + if (join == null) { + join = createOrReuseJoin(JoinEnum.TOKEN, theParamName); + } Predicate singleCode = createPredicateToken(nextOr, theResourceName, theParamName, myBuilder, join); codePredicates.add(singleCode); } @@ -972,38 +996,34 @@ public class SearchBuilder implements ISearchBuilder { @SuppressWarnings("unchecked") private Join createOrReuseJoin(JoinEnum theType, String theSearchParameterName) { - Join join = null; - - switch (theType) { - case DATE: - join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); - break; - case NUMBER: - join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); - break; - case QUANTITY: - join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); - break; - case REFERENCE: - join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); - break; - case STRING: - join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); - break; - case URI: - join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); - break; - case TOKEN: - join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); - break; - } - JoinKey key = new JoinKey(theSearchParameterName, theType); - if (!myIndexJoins.containsKey(key)) { - myIndexJoins.put(key, join); - } - - return (Join) join; + return (Join) myIndexJoins.computeIfAbsent(key, k -> { + Join join = null; + switch (theType) { + case DATE: + join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); + break; + case NUMBER: + join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); + break; + case QUANTITY: + join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); + break; + case REFERENCE: + join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); + break; + case STRING: + join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); + break; + case URI: + join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); + break; + case TOKEN: + join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); + break; + } + return join; + }); } private Predicate createPredicateDate(IQueryParameterType theParam, String theResourceName, String theParamName, CriteriaBuilder theBuilder, From theFrom) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java new file mode 100644 index 00000000000..c4dc2cbe04c --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java @@ -0,0 +1,92 @@ +package ca.uhn.fhir.jpa.config; + +import net.ttddyy.dsproxy.ExecutionInfo; +import net.ttddyy.dsproxy.QueryInfo; +import net.ttddyy.dsproxy.proxy.ParameterSetOperation; +import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; +import org.hibernate.engine.jdbc.internal.BasicFormatterImpl; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; +import java.util.stream.Collectors; + +public class CaptureQueriesListener implements ProxyDataSourceBuilder.SingleQueryExecution { + + private static final LinkedList LAST_N_QUERIES = new LinkedList<>(); + + @Override + public void execute(ExecutionInfo execInfo, List queryInfoList) { + synchronized (LAST_N_QUERIES) { + for (QueryInfo next : queryInfoList) { + String sql = next.getQuery(); + List params; + if (next.getParametersList().size() > 0 && next.getParametersList().get(0).size() > 0) { + List values = next + .getParametersList() + .get(0); + params = values.stream() + .map(t -> t.getArgs()[1]) + .map(t -> t != null ? t.toString() : "NULL") + .collect(Collectors.toList()); + } else { + params = new ArrayList<>(); + } + LAST_N_QUERIES.add(0, new Query(sql, params)); + } + while (LAST_N_QUERIES.size() > 100) { + LAST_N_QUERIES.removeLast(); + } + } + } + + public static class Query { + private final String myThreadName = Thread.currentThread().getName(); + private final String mySql; + private final List myParams; + + Query(String theSql, List theParams) { + mySql = theSql; + myParams = Collections.unmodifiableList(theParams); + } + + public String getThreadName() { + return myThreadName; + } + + public String getSql(boolean theInlineParams, boolean theFormat) { + String retVal = mySql; + if (theFormat) { + retVal = new BasicFormatterImpl().format(retVal); + } + + if (theInlineParams) { + List nextParams = new ArrayList<>(myParams); + while (retVal.contains("?") && nextParams.size() > 0) { + int idx = retVal.indexOf("?"); + retVal = retVal.substring(0, idx) + nextParams.remove(0) + retVal.substring(idx + 1); + } + } + + return retVal; + + } + + } + + public static void clear() { + synchronized (LAST_N_QUERIES) { + LAST_N_QUERIES.clear(); + } + } + + /** + * Index 0 is newest! + */ + public static ArrayList getLastNQueries() { + synchronized (LAST_N_QUERIES) { + return new ArrayList<>(LAST_N_QUERIES); + } + } +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java index d9fcc5773cf..e432dd66a9c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java @@ -100,6 +100,7 @@ public class TestR4Config extends BaseJavaConfigR4 { // .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) // .countQuery(new ThreadQueryCountHolder()) .beforeQuery(new BlockLargeNumbersOfParamsListener()) + .afterQuery(new CaptureQueriesListener()) .countQuery(singleQueryCountHolder()) .build(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java index dacf263e0ac..290c3194f31 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.jpa.config.CaptureQueriesListener; import ca.uhn.fhir.jpa.entity.TermConcept; import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; @@ -94,6 +95,7 @@ public abstract class BaseJpaTest { @After public void afterPerformCleanup() { BaseHapiFhirResourceDao.setDisableIncrementOnUpdateForUnitTest(false); + CaptureQueriesListener.clear(); } @After diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 221fbeaec8c..1491593d61d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao.r4; +import ca.uhn.fhir.jpa.config.CaptureQueriesListener; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap.EverythingModeEnum; @@ -15,6 +16,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; +import org.hibernate.engine.jdbc.internal.BasicFormatterImpl; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -39,6 +41,7 @@ import java.io.IOException; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; import java.util.*; +import java.util.stream.Collectors; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; @@ -2160,6 +2163,51 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } + @Test + public void testSearchLinkToken() { + // /fhirapi/MedicationRequest?category=community&identifier=urn:oid:2.16.840.1.113883.3.7418.12.3%7C&intent=order&medication.code:text=calcitriol,hectorol,Zemplar,rocaltrol,vectical,vitamin%20D,doxercalciferol,paricalcitol&status=active,completed + + Medication m = new Medication(); + m.getCode().setText("valueb"); + myMedicationDao.create(m); + + MedicationRequest mr = new MedicationRequest(); + mr.addCategory().addCoding().setCode("community"); + mr.addIdentifier().setSystem("urn:oid:2.16.840.1.113883.3.7418.12.3").setValue("1"); + mr.setIntent(MedicationRequest.MedicationRequestIntent.ORDER); + mr.setMedication(new Reference(m.getId())); + myMedicationRequestDao.create(mr); + + SearchParameterMap sp = new SearchParameterMap(); + sp.setLoadSynchronous(true); + sp.add("category", new TokenParam("community")); + sp.add("identifier", new TokenParam("urn:oid:2.16.840.1.113883.3.7418.12.3", "1")); + sp.add("intent", new TokenParam("order")); + ReferenceParam param1 = new ReferenceParam("valuea").setChain("code:text"); + ReferenceParam param2 = new ReferenceParam("valueb").setChain("code:text"); + ReferenceParam param3 = new ReferenceParam("valuec").setChain("code:text"); + sp.add("medication", new ReferenceOrListParam().addOr(param1).addOr(param2).addOr(param3)); + + IBundleProvider retrieved = myMedicationRequestDao.search(sp); + assertEquals(1, retrieved.size().intValue()); + + List queries = CaptureQueriesListener + .getLastNQueries() + .stream() + .filter(t->t.getThreadName().equals("main")) + .filter(t -> t.getSql(false, false).toLowerCase().contains("select")) + .filter(t -> t.getSql(false, false).toLowerCase().contains("token")) + .map(t-> t.getSql(true, true)) + .collect(Collectors.toList()); + + ourLog.info("Queries:\n {}", queries.stream().findFirst()); + + String searchQuery = queries.get(0); + assertEquals(searchQuery,3, StringUtils.countMatches(searchQuery.toUpperCase(), "HFJ_SPIDX_TOKEN")); + assertEquals(searchQuery, 5, StringUtils.countMatches(searchQuery.toUpperCase(), "LEFT OUTER JOIN")); + } + + @Test public void testSearchTokenParam() { Patient patient = new Patient(); diff --git a/pom.xml b/pom.xml index e99d52fdaf3..d9795272a1f 100644 --- a/pom.xml +++ b/pom.xml @@ -534,7 +534,7 @@ 9.4.14.v20181114 3.0.2 - 5.4.0.Final + 5.4.1.Final 5.11.0.Final 5.5.5 diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 5de3d69c584..93b4910ffc1 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -333,6 +333,12 @@ whether a call out to the database may be required. I say "may" because subscription matches fail fast so a negative match may be performed in-memory, but a positive match will require a database call. + + When performing a JPA search with a chained :text modifier + (e.g. MedicationStatement?medication.code:text=aspirin,tylenol) a series + of unneccesary joins were introduced to the generated SQL query, harming + performance. This has been fixed. + From 5f29e4fbf337e253c187a6761415ef1ab0f08732 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 30 Jan 2019 05:49:45 -0500 Subject: [PATCH 2/2] Fix #1174 - Prevent serialization exception --- .../ca/uhn/fhir/rest/param/DateParam.java | 10 +++- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 53 +++++++++++++++---- src/changes/changes.xml | 5 ++ 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java index d6c3ec9e9da..246d3b8d099 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java @@ -255,7 +255,15 @@ public class DateParam extends BaseParamWithPrefix implements /*IQuer return b.build(); } - public class DateParamDateTimeHolder extends BaseDateTimeDt { + public static class DateParamDateTimeHolder extends BaseDateTimeDt { + + /** + * Constructor + */ + public DateParamDateTimeHolder() { + super(); + } + @Override protected TemporalPrecisionEnum getDefaultPrecisionForDatatype() { return TemporalPrecisionEnum.SECOND; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 1491593d61d..096aff201a7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -2,9 +2,9 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.config.CaptureQueriesListener; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap.EverythingModeEnum; -import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.util.TestUtil; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; @@ -14,9 +14,9 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.*; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; +import com.google.common.collect.Lists; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; -import org.hibernate.engine.jdbc.internal.BasicFormatterImpl; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -56,6 +56,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); myDaoConfig.setFetchSizeDefaultMaximum(new DaoConfig().getFetchSizeDefaultMaximum()); myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches()); + myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); } @Before @@ -617,7 +618,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { expect1.setResource(resource); expect1.calculateHashes(); - assertThat("Got: \"" + results.toString()+"\"", results, containsInAnyOrder(expect0, expect1)); + assertThat("Got: \"" + results.toString() + "\"", results, containsInAnyOrder(expect0, expect1)); } }); } @@ -1063,7 +1064,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { QuantityParam v1 = new QuantityParam(ParamPrefixEnum.GREATERTHAN_OR_EQUALS, 150, "http://bar", "code1"); SearchParameterMap map = new SearchParameterMap().setLoadSynchronous(true).add(param, v1); IBundleProvider result = myObservationDao.search(map); - assertThat("Got: "+ toUnqualifiedVersionlessIdValues(result), toUnqualifiedVersionlessIdValues(result), containsInAnyOrder(id1.getValue())); + assertThat("Got: " + toUnqualifiedVersionlessIdValues(result), toUnqualifiedVersionlessIdValues(result), containsInAnyOrder(id1.getValue())); } } @@ -1095,7 +1096,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { CompositeParam val = new CompositeParam<>(v0, v1); SearchParameterMap map = new SearchParameterMap().setLoadSynchronous(true).add(param, val); IBundleProvider result = myObservationDao.search(map); - assertThat("Got: "+ toUnqualifiedVersionlessIdValues(result), toUnqualifiedVersionlessIdValues(result), containsInAnyOrder(id2.getValue())); + assertThat("Got: " + toUnqualifiedVersionlessIdValues(result), toUnqualifiedVersionlessIdValues(result), containsInAnyOrder(id2.getValue())); } { TokenParam v0 = new TokenParam("http://foo", "code1"); @@ -1143,6 +1144,40 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } + /** + * See #1174 + */ + @Test + public void testSearchDateInSavedSearch() { + for (int i = 1; i <= 9; i++) { + Patient p1 = new Patient(); + p1.getBirthDateElement().setValueAsString("1980-01-0" + i); + String id1 = myPatientDao.create(p1).getId().toUnqualifiedVersionless().getValue(); + } + + myDaoConfig.setSearchPreFetchThresholds(Lists.newArrayList(3, 6, 10)); + + { + // Don't load synchronous + SearchParameterMap map = new SearchParameterMap(); + map.setLastUpdated(new DateRangeParam().setUpperBound(new DateParam(ParamPrefixEnum.LESSTHAN, "2022-01-01"))); + IBundleProvider found = myPatientDao.search(map); + Set dates = new HashSet<>(); + for (int i = 0; i < 9; i++) { + Patient nextResource = (Patient) found.getResources(i, i + 1).get(0); + dates.add(nextResource.getBirthDateElement().getValueAsString()); + } + + assertThat(dates, hasItems( + "1980-01-01", + "1980-01-09" + )); + + assertFalse(map.isLoadSynchronous()); + assertNull(map.getLoadSynchronousUpTo()); + } + } + /** * #222 */ @@ -2194,16 +2229,16 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { List queries = CaptureQueriesListener .getLastNQueries() .stream() - .filter(t->t.getThreadName().equals("main")) + .filter(t -> t.getThreadName().equals("main")) .filter(t -> t.getSql(false, false).toLowerCase().contains("select")) .filter(t -> t.getSql(false, false).toLowerCase().contains("token")) - .map(t-> t.getSql(true, true)) + .map(t -> t.getSql(true, true)) .collect(Collectors.toList()); ourLog.info("Queries:\n {}", queries.stream().findFirst()); String searchQuery = queries.get(0); - assertEquals(searchQuery,3, StringUtils.countMatches(searchQuery.toUpperCase(), "HFJ_SPIDX_TOKEN")); + assertEquals(searchQuery, 3, StringUtils.countMatches(searchQuery.toUpperCase(), "HFJ_SPIDX_TOKEN")); assertEquals(searchQuery, 5, StringUtils.countMatches(searchQuery.toUpperCase(), "LEFT OUTER JOIN")); } @@ -3362,7 +3397,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { "Observation/YES21", "Observation/YES22", "Observation/YES23" - )); + )); } private void createObservationWithEffective(String theId, String theEffective) { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 93b4910ffc1..a41fac13d20 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -339,6 +339,11 @@ of unneccesary joins were introduced to the generated SQL query, harming performance. This has been fixed. + + A serialization error when performing some searches in the JPA server + using data parameters has been fixed. Thanks to GitHub user + @PickOneFish for reporting! +