4098 Resultset missing resources when querying with _sort result parameter (#4099)

* Add broken test

* Finished writing test

* Add in parameterized test to show it works with one

* Adding extra test asserting sort results when providing short chain

* Providing fix to search with sorting.4098-sort-missing-parameter

* Adding ChangeLog.

* Improving, cleaning up and commenting tests.

Co-authored-by: peartree <etienne.poirier@smilecdr.com>
This commit is contained in:
Tadgh 2022-10-12 05:06:46 -07:00 committed by GitHub
parent 108e357ca7
commit b74e12172e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 211 additions and 50 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 4098
jira: SMILE-5292
title: "Fixed issue where adding a sort parameter to a query would return an incomplete result set."

View File

@ -63,7 +63,6 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.searchparam.extractor.BaseSearchParamExtractor;
import ca.uhn.fhir.jpa.searchparam.util.JpaParamUtil;
import ca.uhn.fhir.jpa.searchparam.util.SourceParam;
import ca.uhn.fhir.jpa.util.QueryParameterUtils;
import ca.uhn.fhir.model.api.IQueryParameterAnd;
import ca.uhn.fhir.model.api.IQueryParameterOr;
import ca.uhn.fhir.model.api.IQueryParameterType;
@ -175,11 +174,13 @@ public class QueryStack {
public void addSortOnDate(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
DatePredicateBuilder sortPredicateBuilder = mySqlBuilder.addDatePredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
DatePredicateBuilder datePredicateBuilder = mySqlBuilder.createDatePredicateBuilder();
Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
mySqlBuilder.addPredicate(hashIdentityPredicate);
mySqlBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending);
Condition hashIdentityPredicate = datePredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, datePredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortDate(datePredicateBuilder.getColumnValueLow(), theAscending);
}
public void addSortOnLastUpdated(boolean theAscending) {
@ -195,22 +196,25 @@ public class QueryStack {
public void addSortOnNumber(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
NumberPredicateBuilder sortPredicateBuilder = mySqlBuilder.addNumberPredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
NumberPredicateBuilder numberPredicateBuilder = mySqlBuilder.createNumberPredicateBuilder();
Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
mySqlBuilder.addPredicate(hashIdentityPredicate);
mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnValue(), theAscending);
Condition hashIdentityPredicate = numberPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, numberPredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortNumeric(numberPredicateBuilder.getColumnValue(), theAscending);
}
public void addSortOnQuantity(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
BaseQuantityPredicateBuilder sortPredicateBuilder;
sortPredicateBuilder = mySqlBuilder.addQuantityPredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
BaseQuantityPredicateBuilder quantityPredicateBuilder = mySqlBuilder.createQuantityPredicateBuilder();
Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
mySqlBuilder.addPredicate(hashIdentityPredicate);
mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnValue(), theAscending);
Condition hashIdentityPredicate = quantityPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, quantityPredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortNumeric(quantityPredicateBuilder.getColumnValue(), theAscending);
}
public void addSortOnResourceId(boolean theAscending) {
@ -227,39 +231,63 @@ public class QueryStack {
public void addSortOnResourceLink(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
ResourceLinkPredicateBuilder sortPredicateBuilder = mySqlBuilder.addReferencePredicateBuilder(this, firstPredicateBuilder.getResourceIdColumn());
ResourceLinkPredicateBuilder resourceLinkPredicateBuilder = mySqlBuilder.createReferencePredicateBuilder(this);
Condition pathPredicate = sortPredicateBuilder.createPredicateSourcePaths(theResourceName, theParamName, new ArrayList<>());
mySqlBuilder.addPredicate(pathPredicate);
mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnTargetResourceId(), theAscending);
Condition pathPredicate = resourceLinkPredicateBuilder.createPredicateSourcePaths(theResourceName, theParamName, new ArrayList<>());
addSortCustomJoin(firstPredicateBuilder, resourceLinkPredicateBuilder, pathPredicate);
mySqlBuilder.addSortNumeric(resourceLinkPredicateBuilder.getColumnTargetResourceId(), theAscending);
}
public void addSortOnString(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
StringPredicateBuilder sortPredicateBuilder = mySqlBuilder.addStringPredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
mySqlBuilder.addPredicate(hashIdentityPredicate);
mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending);
StringPredicateBuilder stringPredicateBuilder = mySqlBuilder.createStringPredicateBuilder();
Condition hashIdentityPredicate = stringPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, stringPredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortString(stringPredicateBuilder.getColumnValueNormalized(), theAscending);
}
public void addSortOnToken(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
TokenPredicateBuilder sortPredicateBuilder = mySqlBuilder.addTokenPredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
mySqlBuilder.addPredicate(hashIdentityPredicate);
mySqlBuilder.addSortString(sortPredicateBuilder.getColumnSystem(), theAscending);
mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValue(), theAscending);
TokenPredicateBuilder tokenPredicateBuilder = mySqlBuilder.createTokenPredicateBuilder();
Condition hashIdentityPredicate = tokenPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, tokenPredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnSystem(), theAscending);
mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnValue(), theAscending);
}
public void addSortOnUri(String theResourceName, String theParamName, boolean theAscending) {
BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
UriPredicateBuilder sortPredicateBuilder = mySqlBuilder.addUriPredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
mySqlBuilder.addPredicate(hashIdentityPredicate);
mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValue(), theAscending);
UriPredicateBuilder uriPredicateBuilder = mySqlBuilder.createUriPredicateBuilder();
Condition hashIdentityPredicate = uriPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, uriPredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortString(uriPredicateBuilder.getColumnValue(), theAscending);
}
private void addSortCustomJoin(BaseJoiningPredicateBuilder theFromJoiningPredicateBuilder, BaseJoiningPredicateBuilder theToJoiningPredicateBuilder, Condition theCondition){
ComboCondition onCondition = mySqlBuilder.createOnCondition(
theFromJoiningPredicateBuilder.getResourceIdColumn(),
theToJoiningPredicateBuilder.getResourceIdColumn()
);
onCondition.addCondition(theCondition);
mySqlBuilder.addCustomJoin(
SelectQuery.JoinType.LEFT_OUTER,
theFromJoiningPredicateBuilder.getTable(),
theToJoiningPredicateBuilder.getTable(),
onCondition);
}
@SuppressWarnings("unchecked")
@ -346,7 +374,6 @@ public class QueryStack {
}
private Condition createMissingParameterQuery(
MissingParameterQueryParams theParams
) {

View File

@ -173,7 +173,7 @@ public class SearchQueryBuilder {
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a DATE search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a DATE search parameter
*/
public DatePredicateBuilder addDatePredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
DatePredicateBuilder retVal = mySqlBuilderFactory.dateIndexTable(this);
@ -181,6 +181,13 @@ public class SearchQueryBuilder {
return retVal;
}
/**
* Create a predicate builder for selecting on a DATE search parameter
*/
public DatePredicateBuilder createDatePredicateBuilder() {
return mySqlBuilderFactory.dateIndexTable(this);
}
/**
* Add and return a predicate builder for selecting a forced ID. This is only intended for use with sorts so it can not
* be the root query.
@ -193,18 +200,24 @@ public class SearchQueryBuilder {
return retVal;
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a NUMBER search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a NUMBER search parameter
*/
public NumberPredicateBuilder addNumberPredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
NumberPredicateBuilder retVal = mySqlBuilderFactory.numberIndexTable(this);
NumberPredicateBuilder retVal = createNumberPredicateBuilder();
addTable(retVal, theSourceJoinColumn);
return retVal;
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a QUANTITY search parameter
* Create a predicate builder for selecting on a NUMBER search parameter
*/
public NumberPredicateBuilder createNumberPredicateBuilder() {
return mySqlBuilderFactory.numberIndexTable(this);
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on the Resource table
*/
public ResourceTablePredicateBuilder addResourceTablePredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
ResourceTablePredicateBuilder retVal = mySqlBuilderFactory.resourceTable(this);
@ -212,18 +225,23 @@ public class SearchQueryBuilder {
return retVal;
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a QUANTITY search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a QUANTITY search parameter
*/
public QuantityPredicateBuilder addQuantityPredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
QuantityPredicateBuilder retVal = mySqlBuilderFactory.quantityIndexTable(this);
QuantityPredicateBuilder retVal = createQuantityPredicateBuilder();
addTable(retVal, theSourceJoinColumn);
return retVal;
}
/**
* Create a predicate builder for selecting on a QUANTITY search parameter
*/
public QuantityPredicateBuilder createQuantityPredicateBuilder() {
return mySqlBuilderFactory.quantityIndexTable(this);
}
public QuantityNormalizedPredicateBuilder addQuantityNormalizedPredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
QuantityNormalizedPredicateBuilder retVal = mySqlBuilderFactory.quantityNormalizedIndexTable(this);
@ -242,16 +260,23 @@ public class SearchQueryBuilder {
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a REFERENCE search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a REFERENCE search parameter
*/
public ResourceLinkPredicateBuilder addReferencePredicateBuilder(QueryStack theQueryStack, @Nullable DbColumn theSourceJoinColumn) {
ResourceLinkPredicateBuilder retVal = mySqlBuilderFactory.referenceIndexTable(theQueryStack, this, false);
ResourceLinkPredicateBuilder retVal = createReferencePredicateBuilder(theQueryStack);
addTable(retVal, theSourceJoinColumn);
return retVal;
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a reosource link where the
* Create a predicate builder for selecting on a REFERENCE search parameter
*/
public ResourceLinkPredicateBuilder createReferencePredicateBuilder(QueryStack theQueryStack) {
return mySqlBuilderFactory.referenceIndexTable(theQueryStack, this, false);
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a resource link where the
* source and target are reversed. This is used for _has queries.
*/
public ResourceLinkPredicateBuilder addReferencePredicateBuilderReversed(QueryStack theQueryStack, DbColumn theSourceJoinColumn) {
@ -261,14 +286,21 @@ public class SearchQueryBuilder {
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a STRING search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a STRING search parameter
*/
public StringPredicateBuilder addStringPredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
StringPredicateBuilder retVal = mySqlBuilderFactory.stringIndexTable(this);
StringPredicateBuilder retVal = createStringPredicateBuilder();
addTable(retVal, theSourceJoinColumn);
return retVal;
}
/**
* Create a predicate builder for selecting on a STRING search parameter
*/
public StringPredicateBuilder createStringPredicateBuilder() {
return mySqlBuilderFactory.stringIndexTable(this);
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a <code>_tag</code> search parameter
*/
@ -279,14 +311,32 @@ public class SearchQueryBuilder {
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a TOKEN search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a TOKEN search parameter
*/
public TokenPredicateBuilder addTokenPredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
TokenPredicateBuilder retVal = mySqlBuilderFactory.tokenIndexTable(this);
TokenPredicateBuilder retVal = createTokenPredicateBuilder();
addTable(retVal, theSourceJoinColumn);
return retVal;
}
/**
* Create a predicate builder for selecting on a TOKEN search parameter
*/
public TokenPredicateBuilder createTokenPredicateBuilder(){
return mySqlBuilderFactory.tokenIndexTable(this);
}
public void addCustomJoin(SelectQuery.JoinType theJoinType, DbTable theFromTable, DbTable theToTable, Condition theCondition) {
mySelect.addCustomJoin(theJoinType, theFromTable, theToTable, theCondition);
}
public ComboCondition createOnCondition(DbColumn theSourceColumn, DbColumn theTargetColumn){
ComboCondition onCondition = ComboCondition.and();
onCondition.addCondition(BinaryCondition.equalTo(theSourceColumn, theTargetColumn));
return onCondition;
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a <code>:missing</code> search parameter
*/
@ -297,14 +347,21 @@ public class SearchQueryBuilder {
}
/**
* Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a URI search parameter
* Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a URI search parameter
*/
public UriPredicateBuilder addUriPredicateBuilder(@Nullable DbColumn theSourceJoinColumn) {
UriPredicateBuilder retVal = mySqlBuilderFactory.uriIndexTable(this);
UriPredicateBuilder retVal = createUriPredicateBuilder();
addTable(retVal, theSourceJoinColumn);
return retVal;
}
/**
* Create a predicate builder for selecting on a URI search parameter
*/
public UriPredicateBuilder createUriPredicateBuilder() {
return mySqlBuilderFactory.uriIndexTable(this);
}
public SqlObjectFactory getSqlBuilderFactory() {
return mySqlBuilderFactory;
}
@ -364,6 +421,12 @@ public class SearchQueryBuilder {
mySelect.addJoins(SelectQuery.JoinType.LEFT_OUTER, join);
}
public void addJoinWithCustomOnCondition(DbTable theFromTable, DbTable theToTable, DbColumn theFromColumn, DbColumn theToColumn, Condition theCondition) {
Join join = new DbJoin(mySpec, theFromTable, theToTable, new DbColumn[]{theFromColumn}, new DbColumn[]{theToColumn});
// add hashIdentity codition here
mySelect.addJoins(SelectQuery.JoinType.LEFT_OUTER, join);
}
/**
* Generate and return the SQL generated by this builder
*/

View File

@ -112,6 +112,8 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.transaction.TransactionDefinition;
import org.springframework.transaction.TransactionStatus;
import org.springframework.transaction.support.TransactionCallbackWithoutResult;
@ -141,6 +143,7 @@ import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.matchesPattern;
import static org.hamcrest.Matchers.not;
@ -3405,6 +3408,69 @@ public class FhirResourceDaoR4Test extends BaseJpaR4Test {
assertThat(actual, contains(id4, id3, id2, id1, idMethodName2, idMethodName1));
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testSortByMissingAttribute(boolean theIndexMissingData) {
myDaoConfig.setIndexMissingFields(theIndexMissingData ? DaoConfig.IndexEnabledEnum.ENABLED : DaoConfig.IndexEnabledEnum.DISABLED);
Patient p = new Patient();
p.setGender(AdministrativeGender.MALE);
myPatientDao.create(p, mySrd);
p = new Patient();
p.setGender(AdministrativeGender.FEMALE);
myPatientDao.create(p, mySrd);
p = new Patient();
myPatientDao.create(p, mySrd);
SearchParameterMap spMap;
List<IIdType> actual;
spMap = SearchParameterMap.newSynchronous();
spMap.setSort(new SortSpec(Patient.SP_GENDER));
myCaptureQueriesListener.clear();
actual = toUnqualifiedVersionlessIds(myPatientDao.search(spMap));
myCaptureQueriesListener.logSelectQueries();
assertEquals(3, actual.size());
myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testSortByMissingAttributeWithChainedSorting(boolean theIndexMissingData) {
myDaoConfig.setIndexMissingFields(theIndexMissingData ? DaoConfig.IndexEnabledEnum.ENABLED : DaoConfig.IndexEnabledEnum.DISABLED);
Patient p = new Patient();
p.addName().addGiven("MalePatientGivenName").setFamily("Bame");
IIdType patient1Id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless();
p = new Patient();
p.setGender(AdministrativeGender.FEMALE);
p.addName().addGiven("FemalePatientGivenName").setFamily("FemalePatientFamilyName");
IIdType patient2Id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless();
p = new Patient();
p.addName().addGiven("MalePatientGivenName").setFamily("Aname");
IIdType patient3Id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless();
SearchParameterMap spMap;
List<IIdType> actual;
spMap = SearchParameterMap.newSynchronous();
spMap.setSort(new SortSpec(Patient.SP_GENDER).setChain(new SortSpec(Patient.SP_NAME)));
myCaptureQueriesListener.clear();
IBundleProvider searchResult = myPatientDao.search(spMap);
actual = toUnqualifiedVersionlessIds(searchResult);
myCaptureQueriesListener.logSelectQueries();
// assert the sorting order
assertThat(actual, hasItems(patient2Id, patient3Id, patient1Id));
myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
}
@Test
public void testSortByLastUpdated() {
String methodName = "testSortByLastUpdated";