Improved the performance for multiple _has by exclude previous pids (#2304)
* Improved the performance for multiple _has by exclude prev pids * Removed extra import * Added test case for SQL contains RES_ID NOT IN ()
This commit is contained in:
parent
e7bc2f0e2d
commit
341eb404e6
|
@ -174,7 +174,8 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
private SqlObjectFactory mySqlBuilderFactory;
|
||||
@Autowired
|
||||
private HibernatePropertiesProvider myDialectProvider;
|
||||
|
||||
|
||||
private boolean hasNextIteratorQuery = false;
|
||||
/**
|
||||
* Constructor
|
||||
*/
|
||||
|
@ -424,6 +425,10 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
sqlBuilder.addPredicate(lastUpdatedPredicates);
|
||||
}
|
||||
|
||||
//-- exclude the pids already in the previous iterator
|
||||
if (hasNextIteratorQuery)
|
||||
sqlBuilder.excludeResourceIdsPredicate(myPidSet);
|
||||
|
||||
/*
|
||||
* Sort
|
||||
*
|
||||
|
@ -436,7 +441,6 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
createSort(queryStack3, sort);
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* Now perform the search
|
||||
*/
|
||||
|
@ -444,7 +448,7 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
if (generatedSql.isMatchNothing()) {
|
||||
return Optional.empty();
|
||||
}
|
||||
|
||||
|
||||
SearchQueryExecutor executor = mySqlBuilderFactory.newSearchQueryExecutor(generatedSql, myMaxResultsToFetch);
|
||||
return Optional.of(executor);
|
||||
}
|
||||
|
@ -1232,8 +1236,10 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
close();
|
||||
if (myQueryList != null && myQueryList.size() > 0) {
|
||||
myResultsIterator = myQueryList.remove(0);
|
||||
hasNextIteratorQuery = true;
|
||||
} else {
|
||||
myResultsIterator = SearchQueryExecutor.emptyExecutor();
|
||||
hasNextIteratorQuery = false;
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -1243,7 +1249,7 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
if (myNext == null) {
|
||||
fetchNext();
|
||||
}
|
||||
return !NO_MORE.equals(myNext);
|
||||
return !NO_MORE.equals(myNext);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -45,6 +45,7 @@ import ca.uhn.fhir.jpa.search.builder.predicate.TokenPredicateBuilder;
|
|||
import ca.uhn.fhir.jpa.search.builder.predicate.UriPredicateBuilder;
|
||||
import ca.uhn.fhir.rest.param.DateRangeParam;
|
||||
import ca.uhn.fhir.rest.param.ParamPrefixEnum;
|
||||
|
||||
import com.healthmarketscience.sqlbuilder.BinaryCondition;
|
||||
import com.healthmarketscience.sqlbuilder.ComboCondition;
|
||||
import com.healthmarketscience.sqlbuilder.Condition;
|
||||
|
@ -66,11 +67,13 @@ import org.hibernate.engine.spi.RowSelection;
|
|||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
|
||||
import javax.annotation.Nonnull;
|
||||
import javax.annotation.Nullable;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
|
@ -542,6 +545,21 @@ public class SearchQueryBuilder {
|
|||
addPredicate(predicate);
|
||||
}
|
||||
|
||||
public void excludeResourceIdsPredicate(Set<ResourcePersistentId> theExsitinghPidSetToExclude) {
|
||||
|
||||
// Do nothing if it's empty
|
||||
if (theExsitinghPidSetToExclude == null || theExsitinghPidSetToExclude.isEmpty())
|
||||
return;
|
||||
|
||||
List<Long> excludePids = ResourcePersistentId.toLongList(theExsitinghPidSetToExclude);
|
||||
|
||||
ourLog.trace("excludePids = " + excludePids);
|
||||
|
||||
DbColumn resourceIdColumn = getOrCreateFirstPredicateBuilder().getResourceIdColumn();
|
||||
InCondition predicate = new InCondition(resourceIdColumn, generatePlaceholders(excludePids));
|
||||
predicate.setNegate(true);
|
||||
addPredicate(predicate);
|
||||
}
|
||||
|
||||
public BinaryCondition createConditionForValueWithComparator(ParamPrefixEnum theComparator, DbColumn theColumn, Object theValue) {
|
||||
switch (theComparator) {
|
||||
|
|
|
@ -3,10 +3,13 @@ package ca.uhn.fhir.jpa.provider.r4;
|
|||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.contains;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotEquals;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.apache.http.client.methods.CloseableHttpResponse;
|
||||
|
@ -19,6 +22,7 @@ import org.hl7.fhir.r4.model.Device;
|
|||
import org.hl7.fhir.r4.model.Encounter;
|
||||
import org.hl7.fhir.r4.model.Observation;
|
||||
import org.hl7.fhir.r4.model.Observation.ObservationComponentComponent;
|
||||
import org.hl7.fhir.r4.model.Observation.ObservationStatus;
|
||||
import org.hl7.fhir.r4.model.Patient;
|
||||
import org.hl7.fhir.r4.model.Period;
|
||||
import org.hl7.fhir.r4.model.Quantity;
|
||||
|
@ -51,7 +55,7 @@ public class ResourceProviderHasParamR4Test extends BaseResourceProviderR4Test {
|
|||
myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds());
|
||||
myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches());
|
||||
myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields());
|
||||
|
||||
|
||||
myClient.unregisterInterceptor(myCapturingInterceptor);
|
||||
}
|
||||
|
||||
|
@ -587,11 +591,36 @@ public class ResourceProviderHasParamR4Test extends BaseResourceProviderR4Test {
|
|||
String uri = ourServerBase + "/Patient?_has:Observation:subject:code-value-quantity=http://" + UrlUtil.escapeUrlParam("loinc.org|2345-7$gt180") + "&_has:Encounter:subject:date=gt1950" + "&_has:Encounter:subject:class=" + UrlUtil.escapeUrlParam("urn:system|IMP");
|
||||
|
||||
ourLog.info("uri = " + uri);
|
||||
|
||||
List<String> ids = searchAndReturnUnqualifiedVersionlessIdValues(uri);
|
||||
|
||||
assertThat(ids, contains(pid0.getValue()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMultipleHasParameter_NOT_IN() throws Exception {
|
||||
|
||||
for (int i=0; i<10; i++) {
|
||||
createPatientWithObs(10);
|
||||
}
|
||||
|
||||
String uri = ourServerBase + "/Patient?_has:Observation:subject:code-value-quantity=http://" + UrlUtil.escapeUrlParam("loinc.org|2345-7$gt180") + "&_has:Observation:subject:date=gt1950" + "&_has:Observation:subject:status=final&_count=4";
|
||||
|
||||
ourLog.info("uri = " + uri);
|
||||
myCaptureQueriesListener.clear();
|
||||
|
||||
searchAndReturnUnqualifiedVersionlessIdValues(uri);
|
||||
|
||||
List<String> queries = myCaptureQueriesListener.getSelectQueries().stream().map(t -> t.getSql(true, false)).collect(Collectors.toList());
|
||||
|
||||
List<String> notInListQueries = new ArrayList<>();
|
||||
for (String query : queries) {
|
||||
if (query.contains("RES_ID NOT IN"))
|
||||
notInListQueries.add(query);
|
||||
}
|
||||
|
||||
assertNotEquals(0, notInListQueries.size());
|
||||
}
|
||||
|
||||
private List<String> searchAndReturnUnqualifiedVersionlessIdValues(String uri) throws IOException {
|
||||
List<String> ids;
|
||||
HttpGet get = new HttpGet(uri);
|
||||
|
@ -605,5 +634,26 @@ public class ResourceProviderHasParamR4Test extends BaseResourceProviderR4Test {
|
|||
return ids;
|
||||
}
|
||||
|
||||
|
||||
private void createPatientWithObs(int obsNum) {
|
||||
|
||||
Patient patient = new Patient();
|
||||
patient.addIdentifier().setSystem("urn:system").setValue("001");
|
||||
patient.addName().setFamily("Tester").addGiven("Joe");
|
||||
IIdType pid = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless();
|
||||
|
||||
Observation o1 = new Observation();
|
||||
o1.setStatus(ObservationStatus.FINAL);
|
||||
o1.getSubject().setReferenceElement(pid);
|
||||
o1.setEffective(new DateTimeType("2001-02-01"));
|
||||
CodeableConcept cc = o1.getCode();
|
||||
cc.addCoding().setCode("2345-7").setSystem("http://loinc.org");
|
||||
o1.setValue(new Quantity().setValue(200));
|
||||
cc = new CodeableConcept();
|
||||
cc.addCoding().setCode("2345-7").setSystem("http://loinc.org");
|
||||
o1.addCategory(cc);
|
||||
|
||||
for (int i=0; i<obsNum; i++) {
|
||||
myObservationDao.create(o1).getId().toUnqualifiedVersionless();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue