diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java index 2bd614caaf0..57d6aa929c5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java @@ -146,6 +146,8 @@ public class QueryStack { private final DaoConfig myDaoConfig; private final EnumSet myReusePredicateBuilderTypes; private Map myJoinMap; + // used for _offset queries with sort, should be removed once the fix is applied to the async path too. + private boolean myUseAggregate; /** * Constructor @@ -182,7 +184,7 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, datePredicateBuilder, hashIdentityPredicate); - mySqlBuilder.addSortDate(datePredicateBuilder.getColumnValueLow(), theAscending); + mySqlBuilder.addSortDate(datePredicateBuilder.getColumnValueLow(), theAscending, myUseAggregate); } public void addSortOnLastUpdated(boolean theAscending) { @@ -193,7 +195,7 @@ public class QueryStack { } else { resourceTablePredicateBuilder = mySqlBuilder.addResourceTablePredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); } - mySqlBuilder.addSortDate(resourceTablePredicateBuilder.getColumnLastUpdated(), theAscending); + mySqlBuilder.addSortDate(resourceTablePredicateBuilder.getColumnLastUpdated(), theAscending, myUseAggregate); } public void addSortOnNumber(String theResourceName, String theParamName, boolean theAscending) { @@ -204,7 +206,7 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, numberPredicateBuilder, hashIdentityPredicate); - mySqlBuilder.addSortNumeric(numberPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortNumeric(numberPredicateBuilder.getColumnValue(), theAscending, myUseAggregate); } public void addSortOnQuantity(String theResourceName, String theParamName, boolean theAscending) { @@ -216,18 +218,18 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, quantityPredicateBuilder, hashIdentityPredicate); - mySqlBuilder.addSortNumeric(quantityPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortNumeric(quantityPredicateBuilder.getColumnValue(), theAscending, myUseAggregate); } public void addSortOnResourceId(boolean theAscending) { BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); ForcedIdPredicateBuilder sortPredicateBuilder = mySqlBuilder.addForcedIdPredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); if (!theAscending) { - mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), false, OrderObject.NullOrder.FIRST); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), false, OrderObject.NullOrder.FIRST, myUseAggregate); } else { - mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), true); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), true, myUseAggregate); } - mySqlBuilder.addSortNumeric(firstPredicateBuilder.getResourceIdColumn(), theAscending); + mySqlBuilder.addSortNumeric(firstPredicateBuilder.getResourceIdColumn(), theAscending, myUseAggregate); } @@ -239,7 +241,7 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, resourceLinkPredicateBuilder, pathPredicate); - mySqlBuilder.addSortNumeric(resourceLinkPredicateBuilder.getColumnTargetResourceId(), theAscending); + mySqlBuilder.addSortNumeric(resourceLinkPredicateBuilder.getColumnTargetResourceId(), theAscending, myUseAggregate); } public void addSortOnString(String theResourceName, String theParamName, boolean theAscending) { @@ -250,7 +252,7 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, stringPredicateBuilder, hashIdentityPredicate); - mySqlBuilder.addSortString(stringPredicateBuilder.getColumnValueNormalized(), theAscending); + mySqlBuilder.addSortString(stringPredicateBuilder.getColumnValueNormalized(), theAscending, myUseAggregate); } public void addSortOnToken(String theResourceName, String theParamName, boolean theAscending) { @@ -261,8 +263,8 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, tokenPredicateBuilder, hashIdentityPredicate); - mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnSystem(), theAscending); - mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnSystem(), theAscending, myUseAggregate); + mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnValue(), theAscending, myUseAggregate); } @@ -274,7 +276,7 @@ public class QueryStack { addSortCustomJoin(firstPredicateBuilder, uriPredicateBuilder, hashIdentityPredicate); - mySqlBuilder.addSortString(uriPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortString(uriPredicateBuilder.getColumnValue(), theAscending, myUseAggregate); } private void addSortCustomJoin(BaseJoiningPredicateBuilder theFromJoiningPredicateBuilder, BaseJoiningPredicateBuilder theToJoiningPredicateBuilder, Condition theCondition){ @@ -292,6 +294,10 @@ public class QueryStack { onCondition); } + public void setUseAggregate(boolean theUseAggregate) { + myUseAggregate = theUseAggregate; + } + @SuppressWarnings("unchecked") private PredicateBuilderCacheLookupResult createOrReusePredicateBuilder(PredicateBuilderTypeEnum theType, DbColumn theSourceJoinColumn, String theParamName, Supplier theFactoryMethod) { boolean cacheHit = false; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 168ce334887..9725f930027 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -636,6 +636,14 @@ public class SearchBuilder implements ISearchBuilder { } } + /* + * If offset is present, we want deduplicate the results by using GROUP BY + */ + if (theOffset != null) { + queryStack3.addGrouping(); + queryStack3.setUseAggregate(true); + } + /* * Sort * @@ -648,9 +656,6 @@ public class SearchBuilder implements ISearchBuilder { createSort(queryStack3, sort); } - if (theOffset != null) { - queryStack3.addGrouping(); - } /* * Now perform the search diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java index 1dc560cd2f7..9fe34698b6e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java @@ -79,7 +79,11 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; -import static ca.uhn.fhir.rest.param.ParamPrefixEnum.*; +import static ca.uhn.fhir.rest.param.ParamPrefixEnum.GREATERTHAN; +import static ca.uhn.fhir.rest.param.ParamPrefixEnum.GREATERTHAN_OR_EQUALS; +import static ca.uhn.fhir.rest.param.ParamPrefixEnum.LESSTHAN; +import static ca.uhn.fhir.rest.param.ParamPrefixEnum.LESSTHAN_OR_EQUALS; +import static ca.uhn.fhir.rest.param.ParamPrefixEnum.NOT_EQUAL; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; public class SearchQueryBuilder { @@ -125,10 +129,10 @@ public class SearchQueryBuilder { mySqlBuilderFactory = theSqlBuilderFactory; myCountQuery = theCountQuery; myDialect = theDialect; - if (myDialect instanceof org.hibernate.dialect.MySQLDialect){ + if (myDialect instanceof org.hibernate.dialect.MySQLDialect) { dialectIsMySql = true; } - if (myDialect instanceof org.hibernate.dialect.SQLServerDialect){ + if (myDialect instanceof org.hibernate.dialect.SQLServerDialect) { dialectIsMsSql = true; } @@ -272,7 +276,7 @@ public class SearchQueryBuilder { * Create a predicate builder for selecting on a REFERENCE search parameter */ public ResourceLinkPredicateBuilder createReferencePredicateBuilder(QueryStack theQueryStack) { - return mySqlBuilderFactory.referenceIndexTable(theQueryStack, this, false); + return mySqlBuilderFactory.referenceIndexTable(theQueryStack, this, false); } /** @@ -322,7 +326,7 @@ public class SearchQueryBuilder { /** * Create a predicate builder for selecting on a TOKEN search parameter */ - public TokenPredicateBuilder createTokenPredicateBuilder(){ + public TokenPredicateBuilder createTokenPredicateBuilder() { return mySqlBuilderFactory.tokenIndexTable(this); } @@ -330,7 +334,7 @@ public class SearchQueryBuilder { mySelect.addCustomJoin(theJoinType, theFromTable, theToTable, theCondition); } - public ComboCondition createOnCondition(DbColumn theSourceColumn, DbColumn theTargetColumn){ + public ComboCondition createOnCondition(DbColumn theSourceColumn, DbColumn theTargetColumn) { ComboCondition onCondition = ComboCondition.and(); onCondition.addCondition(BinaryCondition.equalTo(theSourceColumn, theTargetColumn)); @@ -359,7 +363,7 @@ public class SearchQueryBuilder { * Create a predicate builder for selecting on a URI search parameter */ public UriPredicateBuilder createUriPredicateBuilder() { - return mySqlBuilderFactory.uriIndexTable(this); + return mySqlBuilderFactory.uriIndexTable(this); } public SqlObjectFactory getSqlBuilderFactory() { @@ -709,21 +713,33 @@ public class SearchQueryBuilder { } public void addSortString(DbColumn theColumnValueNormalized, boolean theAscending) { + addSortString(theColumnValueNormalized, theAscending, false); + } + + public void addSortString(DbColumn theColumnValueNormalized, boolean theAscending, boolean theUseAggregate) { OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; - addSortString(theColumnValueNormalized, theAscending, nullOrder); + addSortString(theColumnValueNormalized, theAscending, nullOrder, theUseAggregate); } public void addSortNumeric(DbColumn theColumnValueNormalized, boolean theAscending) { + addSortNumeric(theColumnValueNormalized, theAscending, false); + } + + public void addSortNumeric(DbColumn theColumnValueNormalized, boolean theAscending, boolean theUseAggregate) { OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; - addSortNumeric(theColumnValueNormalized, theAscending, nullOrder); + addSortNumeric(theColumnValueNormalized, theAscending, nullOrder, theUseAggregate); } public void addSortDate(DbColumn theColumnValueNormalized, boolean theAscending) { - OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; - addSortDate(theColumnValueNormalized, theAscending, nullOrder); + addSortDate(theColumnValueNormalized, theAscending, false); } - public void addSortString(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + public void addSortDate(DbColumn theColumnValueNormalized, boolean theAscending, boolean theUseAggregate) { + OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; + addSortDate(theColumnValueNormalized, theAscending, nullOrder, theUseAggregate); + } + + public void addSortString(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder, boolean theUseAggregate) { if ((dialectIsMySql || dialectIsMsSql)) { // MariaDB, MySQL and MSSQL do not support "NULLS FIRST" and "NULLS LAST" syntax. String direction = theTheAscending ? " ASC" : " DESC"; @@ -740,14 +756,28 @@ public class SearchQueryBuilder { sortColumnNameBuilder.append( "CASE WHEN " ).append( sortColumnName ).append( " IS NULL THEN 1 ELSE 0 END" ).append(direction).append(", "); } */ + sortColumnName = formatColumnNameForAggregate(theTheAscending, theUseAggregate, sortColumnName); sortColumnNameBuilder.append(sortColumnName).append(direction); mySelect.addCustomOrderings(sortColumnNameBuilder.toString()); } else { - addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder); + addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder, theUseAggregate); } } - public void addSortNumeric(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + private static String formatColumnNameForAggregate(boolean theTheAscending, boolean theUseAggregate, String sortColumnName) { + if (theUseAggregate) { + String aggregateFunction; + if (theTheAscending) { + aggregateFunction = "MIN"; + } else { + aggregateFunction = "MAX"; + } + sortColumnName = aggregateFunction + "(" + sortColumnName + ")"; + } + return sortColumnName; + } + + public void addSortNumeric(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder, boolean theUseAggregate) { if ((dialectIsMySql || dialectIsMsSql)) { // MariaDB, MySQL and MSSQL do not support "NULLS FIRST" and "NULLS LAST" syntax. // Null values are always treated as less than non-null values. @@ -763,13 +793,14 @@ public class SearchQueryBuilder { } else { direction = theTheAscending ? " ASC" : " DESC"; } + sortColumnName = formatColumnNameForAggregate(theTheAscending, theUseAggregate, sortColumnName); mySelect.addCustomOrderings(sortColumnName + direction); } else { - addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder); + addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder, theUseAggregate); } } - public void addSortDate(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + public void addSortDate(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder, boolean theUseAggregate) { if ((dialectIsMySql || dialectIsMsSql)) { // MariaDB, MySQL and MSSQL do not support "NULLS FIRST" and "NULLS LAST" syntax. String direction = theTheAscending ? " ASC" : " DESC"; @@ -786,16 +817,25 @@ public class SearchQueryBuilder { sortColumnNameBuilder.append( "CASE WHEN " ).append( sortColumnName ).append( " IS NULL THEN 1 ELSE 0 END" ).append(direction).append(", "); } */ + sortColumnName = formatColumnNameForAggregate(theTheAscending, theUseAggregate, sortColumnName); sortColumnNameBuilder.append(sortColumnName).append(direction); mySelect.addCustomOrderings(sortColumnNameBuilder.toString()); } else { - addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder); + addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder, theUseAggregate); } } - private void addSort(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + private void addSort(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder, boolean theUseAggregate) { OrderObject.Dir direction = theTheAscending ? OrderObject.Dir.ASCENDING : OrderObject.Dir.DESCENDING; - OrderObject orderObject = new OrderObject(direction, theTheColumnValueNormalized); + Object columnToOrder = theTheColumnValueNormalized; + if (theUseAggregate) { + if (theTheAscending) { + columnToOrder = FunctionCall.min().addColumnParams(theTheColumnValueNormalized); + } else { + columnToOrder = FunctionCall.max().addColumnParams(theTheColumnValueNormalized); + } + } + OrderObject orderObject = new OrderObject(direction, columnToOrder); orderObject.setNullOrder(theNullOrder); mySelect.addCustomOrderings(orderObject); } @@ -804,7 +844,7 @@ public class SearchQueryBuilder { * If set to true (default is false), force the generated SQL to start * with the {@link ca.uhn.fhir.jpa.model.entity.ResourceTable HFJ_RESOURCE} * table at the root of the query. - * + *

* This seems to perform better if there are multiple joins on the * resource ID table. */ diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/BaseSearchQueryBuilderDialectTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/BaseSearchQueryBuilderDialectTest.java index 19c8e577c0b..aae795957b2 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/BaseSearchQueryBuilderDialectTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/BaseSearchQueryBuilderDialectTest.java @@ -57,7 +57,7 @@ public abstract class BaseSearchQueryBuilderDialectTest { if (theNullOrder == null) { searchQueryBuilder.addSortNumeric(sortPredicateBuilder.getColumnValueLow(), theAscending); } else { - searchQueryBuilder.addSortNumeric(sortPredicateBuilder.getColumnValueLow(), theAscending, theNullOrder); + searchQueryBuilder.addSortNumeric(sortPredicateBuilder.getColumnValueLow(), theAscending, theNullOrder, false); } return searchQueryBuilder.generate(0, 500); diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderDialectMySqlTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderDialectMySqlTest.java index 5d222e7dd98..af0b511a83e 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderDialectMySqlTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderDialectMySqlTest.java @@ -70,7 +70,7 @@ public class SearchQueryBuilderDialectMySqlTest extends BaseSearchQueryBuilderDi if (theNullOrder == null) { searchQueryBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending); } else { - searchQueryBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending, theNullOrder); + searchQueryBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending, theNullOrder, false); } return searchQueryBuilder.generate(0,500); @@ -119,7 +119,7 @@ public class SearchQueryBuilderDialectMySqlTest extends BaseSearchQueryBuilderDi if (theNullOrder == null) { searchQueryBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending); } else { - searchQueryBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending, theNullOrder); + searchQueryBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending, theNullOrder, false); } return searchQueryBuilder.generate(0,500);