From 9143f5995bece17591e95e607e05f56f1eef1306 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 28 Jun 2024 14:43:33 -0400 Subject: [PATCH] Combo permutations --- .../hapi/fhir/docs/server_jpa/upgrading.md | 1 + .../fhir/jpa/search/builder/QueryStack.java | 8 +- .../jpa/search/builder/SearchBuilder.java | 180 +++++---- ...UniqueSearchParameterPredicateBuilder.java | 23 +- ...UniqueSearchParameterPredicateBuilder.java | 12 +- .../uhn/fhir/jpa/util/PermutationBuilder.java | 101 +++++ .../fhir/jpa/util/PermutationBuilderTest.java | 63 ++++ ...rResourceDaoR4ComboNonUniqueParamTest.java | 352 +++++++++++++++--- .../FhirResourceDaoR4ComboUniqueParamIT.java | 52 ++- 9 files changed, 633 insertions(+), 159 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md index b7afafd1358..db88f84a419 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/upgrading.md @@ -21,6 +21,7 @@ You may use the following command to get detailed help on the options: Note the arguments: * `-d [dialect]` – This indicates the database dialect to use. See the detailed help for a list of options +* `--enable-heavyweight-migrations` – If this flag is set, additional migration tasks will be executed that are considered unnecessary to execute on a database with a significant amount of data loaded. This option is not generally necessary. # Oracle Support 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 685f96c16f8..99ff10640b9 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 @@ -2779,16 +2779,16 @@ public class QueryStack { } } - public void addPredicateCompositeUnique(String theIndexString, RequestPartitionId theRequestPartitionId) { + public void addPredicateCompositeUnique(List theIndexStrings, RequestPartitionId theRequestPartitionId) { ComboUniqueSearchParameterPredicateBuilder predicateBuilder = mySqlBuilder.addComboUniquePredicateBuilder(); - Condition predicate = predicateBuilder.createPredicateIndexString(theRequestPartitionId, theIndexString); + Condition predicate = predicateBuilder.createPredicateIndexString(theRequestPartitionId, theIndexStrings); mySqlBuilder.addPredicate(predicate); } - public void addPredicateCompositeNonUnique(String theIndexString, RequestPartitionId theRequestPartitionId) { + public void addPredicateCompositeNonUnique(List theIndexStrings, RequestPartitionId theRequestPartitionId) { ComboNonUniqueSearchParameterPredicateBuilder predicateBuilder = mySqlBuilder.addComboNonUniquePredicateBuilder(); - Condition predicate = predicateBuilder.createPredicateHashComplete(theRequestPartitionId, theIndexString); + Condition predicate = predicateBuilder.createPredicateHashComplete(theRequestPartitionId, theIndexStrings); mySqlBuilder.addPredicate(predicate); } 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 80b6fd51b6c..4161ff5f290 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 @@ -69,6 +69,7 @@ import ca.uhn.fhir.jpa.searchparam.util.JpaParamUtil; import ca.uhn.fhir.jpa.searchparam.util.LastNParameterHelper; import ca.uhn.fhir.jpa.util.BaseIterator; import ca.uhn.fhir.jpa.util.CurrentThreadCaptureQueriesListener; +import ca.uhn.fhir.jpa.util.PermutationBuilder; import ca.uhn.fhir.jpa.util.QueryChunker; import ca.uhn.fhir.jpa.util.SqlQueryList; import ca.uhn.fhir.model.api.IQueryParameterType; @@ -83,6 +84,7 @@ import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails; import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.param.BaseParamWithPrefix; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.ParameterUtil; @@ -125,6 +127,7 @@ import org.springframework.transaction.support.TransactionSynchronizationManager import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -137,6 +140,7 @@ import java.util.stream.Collectors; import static ca.uhn.fhir.jpa.model.util.JpaConstants.UNDESIRED_RESOURCE_LINKAGES_FOR_EVERYTHING_ON_PATIENT_INSTANCE; import static ca.uhn.fhir.jpa.search.builder.QueryStack.LOCATION_POSITION; import static ca.uhn.fhir.jpa.search.builder.QueryStack.SearchForIdsParams.with; +import static java.util.Objects.requireNonNull; import static org.apache.commons.lang3.StringUtils.defaultString; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -1910,10 +1914,12 @@ public class SearchBuilder implements ISearchBuilder { if (comboParam != null) { Collections.sort(comboParamNames); - if (!validateParamValuesAreValidForComboParam(theParams, comboParamNames)) { + if (!validateParamValuesAreValidForComboParam(theRequest, theParams, comboParamNames)) { return; } + // FIXME: abort if too many permutations + applyComboSearchParam(theQueryStack, theParams, theRequest, comboParamNames, comboParam); } } @@ -1927,101 +1933,111 @@ public class SearchBuilder implements ISearchBuilder { // Since we're going to remove elements below theParams.values().forEach(this::ensureSubListsAreWritable); - StringBuilder theSearchBuilder = new StringBuilder(); - theSearchBuilder.append(myResourceName); - theSearchBuilder.append("?"); - - boolean first = true; + List> inputs = new ArrayList<>(); for (String nextParamName : theComboParamNames) { - List> nextValues = theParams.get(nextParamName); + List nextValues = theParams.get(nextParamName).remove(0); + inputs.add(nextValues); + } - // This should never happen, but this safety check was added along the way and - // presumably must save us in some specific race condition. I am preserving it - // in a refactor of this code base. 20240429 - if (nextValues.isEmpty()) { - ourLog.error( - "query parameter {} is unexpectedly empty. Encountered while considering {} index for {}", - nextParamName, - theComboParam.getName(), - theRequest.getCompleteUrl()); - continue; - } + List> inputPermutations = PermutationBuilder.calculatePermutations(inputs); + List indexStrings = new ArrayList<>(PermutationBuilder.calculatePermutationCount(inputs)); + for (List nextPermutation : inputPermutations) { - List nextAnd = nextValues.remove(0); - IQueryParameterType nextOr = nextAnd.remove(0); - String nextOrValue = nextOr.getValueAsQueryToken(myContext); + StringBuilder searchStringBuilder = new StringBuilder(); + searchStringBuilder.append(myResourceName); + searchStringBuilder.append("?"); - RuntimeSearchParam nextParamDef = mySearchParamRegistry.getActiveSearchParam(myResourceName, nextParamName); - if (theComboParam.getComboSearchParamType() == ComboSearchParamType.NON_UNIQUE) { - if (nextParamDef.getParamType() == RestSearchParameterTypeEnum.STRING) { - nextOrValue = StringUtil.normalizeStringForSearchIndexing(nextOrValue); + boolean first = true; + for (int paramIndex = 0; paramIndex < theComboParamNames.size(); paramIndex++) { + + String nextParamName = theComboParamNames.get(paramIndex); + IQueryParameterType nextOr = nextPermutation.get(paramIndex); + String nextOrValue = nextOr.getValueAsQueryToken(myContext); + + RuntimeSearchParam nextParamDef = mySearchParamRegistry.getActiveSearchParam(myResourceName, nextParamName); + if (theComboParam.getComboSearchParamType() == ComboSearchParamType.NON_UNIQUE) { + if (nextParamDef.getParamType() == RestSearchParameterTypeEnum.STRING) { + nextOrValue = StringUtil.normalizeStringForSearchIndexing(nextOrValue); + } } + + if (first) { + first = false; + } else { + searchStringBuilder.append('&'); + } + + nextParamName = UrlUtil.escapeUrlParam(nextParamName); + nextOrValue = UrlUtil.escapeUrlParam(nextOrValue); + + searchStringBuilder.append(nextParamName).append('=').append(nextOrValue); } - if (first) { - first = false; - } else { - theSearchBuilder.append('&'); - } - - nextParamName = UrlUtil.escapeUrlParam(nextParamName); - nextOrValue = UrlUtil.escapeUrlParam(nextOrValue); - - theSearchBuilder.append(nextParamName).append('=').append(nextOrValue); - } - - if (theSearchBuilder != null) { - String indexString = theSearchBuilder.toString(); + String indexString = searchStringBuilder.toString(); ourLog.debug( - "Checking for {} combo index for query: {}", theComboParam.getComboSearchParamType(), indexString); + "Checking for {} combo index for query: {}", theComboParam.getComboSearchParamType(), indexString); - // Interceptor broadcast: JPA_PERFTRACE_INFO - StorageProcessingMessage msg = new StorageProcessingMessage() - .setMessage("Using " + theComboParam.getComboSearchParamType() + " index for query for search: " - + indexString); - HookParams params = new HookParams() - .add(RequestDetails.class, theRequest) - .addIfMatchesType(ServletRequestDetails.class, theRequest) - .add(StorageProcessingMessage.class, msg); - CompositeInterceptorBroadcaster.doCallHooks( - myInterceptorBroadcaster, theRequest, Pointcut.JPA_PERFTRACE_INFO, params); - - switch (theComboParam.getComboSearchParamType()) { - case UNIQUE: - theQueryStack.addPredicateCompositeUnique(indexString, myRequestPartitionId); - break; - case NON_UNIQUE: - theQueryStack.addPredicateCompositeNonUnique(indexString, myRequestPartitionId); - break; - } - - // Remove any empty parameters remaining after this - theParams.clean(); + indexStrings.add(indexString); } + + // Just to make sure we're stable for tests + indexStrings.sort(Comparator.naturalOrder()); + + // Interceptor broadcast: JPA_PERFTRACE_INFO + String indexStringForLog = indexStrings.size() > 1 ? indexStrings.toString() : indexStrings.get(0); + StorageProcessingMessage msg = new StorageProcessingMessage() + .setMessage("Using " + theComboParam.getComboSearchParamType() + " index(es) for query for search: " + + indexStringForLog); + HookParams params = new HookParams() + .add(RequestDetails.class, theRequest) + .addIfMatchesType(ServletRequestDetails.class, theRequest) + .add(StorageProcessingMessage.class, msg); + CompositeInterceptorBroadcaster.doCallHooks( + myInterceptorBroadcaster, theRequest, Pointcut.JPA_PERFTRACE_INFO, params); + + switch (requireNonNull(theComboParam.getComboSearchParamType())) { + case UNIQUE: + theQueryStack.addPredicateCompositeUnique(indexStrings, myRequestPartitionId); + break; + case NON_UNIQUE: + theQueryStack.addPredicateCompositeNonUnique(indexStrings, myRequestPartitionId); + break; + } + + // Remove any empty parameters remaining after this + theParams.clean(); } private boolean validateParamValuesAreValidForComboParam( - @Nonnull SearchParameterMap theParams, List comboParamNames) { + RequestDetails theRequest, @Nonnull SearchParameterMap theParams, List comboParamNames) { boolean paramValuesAreValidForCombo = true; for (String nextParamName : comboParamNames) { List> nextValues = theParams.get(nextParamName); - // Multiple AND parameters are not supported for unique combo params - if (nextValues.get(0).size() != 1) { - ourLog.debug( - "Search is not a candidate for unique combo searching - Multiple AND expressions found for the same parameter"); + if (nextValues.isEmpty()) { paramValuesAreValidForCombo = false; break; } - List nextAndValue = nextValues.get(0); - for (IQueryParameterType nextOrValue : nextAndValue) { - if (nextOrValue instanceof DateParam) { - if (((DateParam) nextOrValue).getPrecision() != TemporalPrecisionEnum.DAY) { - ourLog.debug( - "Search is not a candidate for unique combo searching - Date search with non-DAY precision"); - paramValuesAreValidForCombo = false; - break; + for (List nextAndValue : nextValues) { + for (IQueryParameterType nextOrValue : nextAndValue) { + if (nextOrValue instanceof DateParam) { + DateParam dateParam = (DateParam) nextOrValue; + if (dateParam.getPrecision() != TemporalPrecisionEnum.DAY) { + String message = "Search with params " + comboParamNames + " is not a candidate for combo searching - Date search with non-DAY precision for parameter '" + nextParamName + "'"; + firePerformanceInfo(theRequest, message); + paramValuesAreValidForCombo = false; + break; + } + } + if (nextOrValue instanceof BaseParamWithPrefix) { + BaseParamWithPrefix paramWithPrefix = (BaseParamWithPrefix) nextOrValue; + if (paramWithPrefix.getPrefix() != null) { + String message = "Search with params " + comboParamNames + " is not a candidate for combo searching - Parameter '" + nextParamName + "' has prefix: '" + paramWithPrefix.getPrefix().getValue() + "'"; + firePerformanceInfo(theRequest, message); + paramValuesAreValidForCombo = false; + break; + } } } } @@ -2416,8 +2432,20 @@ public class SearchBuilder implements ISearchBuilder { } } + private void firePerformanceInfo(RequestDetails theRequest, String theMessage) { + // Only log at debug level since these messages aren't considered important enough + // that we should be cluttering the system log, but they are important to the + // specific query being executed to we'll INFO level them there + ourLog.debug(theMessage); + firePerformanceMessage(theRequest, theMessage, Pointcut.JPA_PERFTRACE_INFO); + } + private void firePerformanceWarning(RequestDetails theRequest, String theMessage) { ourLog.warn(theMessage); + firePerformanceMessage(theRequest, theMessage, Pointcut.JPA_PERFTRACE_WARNING); + } + + private void firePerformanceMessage(RequestDetails theRequest, String theMessage, Pointcut pointcut) { StorageProcessingMessage message = new StorageProcessingMessage(); message.setMessage(theMessage); HookParams params = new HookParams() @@ -2425,7 +2453,7 @@ public class SearchBuilder implements ISearchBuilder { .addIfMatchesType(ServletRequestDetails.class, theRequest) .add(StorageProcessingMessage.class, message); CompositeInterceptorBroadcaster.doCallHooks( - myInterceptorBroadcaster, theRequest, Pointcut.JPA_PERFTRACE_WARNING, params); + myInterceptorBroadcaster, theRequest, pointcut, params); } public static int getMaximumPageSize() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboNonUniqueSearchParameterPredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboNonUniqueSearchParameterPredicateBuilder.java index af6dc0074a7..9f144f30daf 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboNonUniqueSearchParameterPredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboNonUniqueSearchParameterPredicateBuilder.java @@ -25,8 +25,12 @@ import ca.uhn.fhir.jpa.model.entity.ResourceIndexedComboTokenNonUnique; import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; import com.healthmarketscience.sqlbuilder.BinaryCondition; import com.healthmarketscience.sqlbuilder.Condition; +import com.healthmarketscience.sqlbuilder.InCondition; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbColumn; +import java.util.List; +import java.util.stream.Collectors; + public class ComboNonUniqueSearchParameterPredicateBuilder extends BaseSearchParamPredicateBuilder { private final DbColumn myColumnHashComplete; @@ -40,12 +44,21 @@ public class ComboNonUniqueSearchParameterPredicateBuilder extends BaseSearchPar myColumnHashComplete = getTable().addColumn("HASH_COMPLETE"); } - public Condition createPredicateHashComplete(RequestPartitionId theRequestPartitionId, String theIndexString) { + public Condition createPredicateHashComplete(RequestPartitionId theRequestPartitionId, List theIndexStrings) { PartitionablePartitionId partitionId = - PartitionablePartitionId.toStoragePartition(theRequestPartitionId, getPartitionSettings()); - long hash = ResourceIndexedComboTokenNonUnique.calculateHashComplete( - getPartitionSettings(), partitionId, theIndexString); - BinaryCondition predicate = BinaryCondition.equalTo(myColumnHashComplete, generatePlaceholder(hash)); + PartitionablePartitionId.toStoragePartition(theRequestPartitionId, getPartitionSettings()); + Condition predicate; + if (theIndexStrings.size() == 1) { + long hash = ResourceIndexedComboTokenNonUnique.calculateHashComplete( + getPartitionSettings(), partitionId, theIndexStrings.get(0)); + predicate = BinaryCondition.equalTo(myColumnHashComplete, generatePlaceholder(hash)); + } else { + List hashes = theIndexStrings + .stream() + .map(t -> ResourceIndexedComboTokenNonUnique.calculateHashComplete(getPartitionSettings(), partitionId, t)) + .collect(Collectors.toList()); + predicate = new InCondition(myColumnHashComplete, generatePlaceholders(hashes)); + } return combineWithRequestPartitionIdPredicate(theRequestPartitionId, predicate); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboUniqueSearchParameterPredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboUniqueSearchParameterPredicateBuilder.java index a65fc9b98ff..7a5d677fd39 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboUniqueSearchParameterPredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ComboUniqueSearchParameterPredicateBuilder.java @@ -23,8 +23,11 @@ import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; import com.healthmarketscience.sqlbuilder.BinaryCondition; import com.healthmarketscience.sqlbuilder.Condition; +import com.healthmarketscience.sqlbuilder.InCondition; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbColumn; +import java.util.List; + public class ComboUniqueSearchParameterPredicateBuilder extends BaseSearchParamPredicateBuilder { private final DbColumn myColumnString; @@ -38,8 +41,13 @@ public class ComboUniqueSearchParameterPredicateBuilder extends BaseSearchParamP myColumnString = getTable().addColumn("IDX_STRING"); } - public Condition createPredicateIndexString(RequestPartitionId theRequestPartitionId, String theIndexString) { - BinaryCondition predicate = BinaryCondition.equalTo(myColumnString, generatePlaceholder(theIndexString)); + public Condition createPredicateIndexString(RequestPartitionId theRequestPartitionId, List theIndexStrings) { + Condition predicate; + if (theIndexStrings.size() == 1) { + predicate = BinaryCondition.equalTo(myColumnString, generatePlaceholder(theIndexStrings.get(0))); + } else { + predicate = new InCondition(myColumnString, generatePlaceholders(theIndexStrings)); + } return combineWithRequestPartitionIdPredicate(theRequestPartitionId, predicate); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java new file mode 100644 index 00000000000..331112b1931 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java @@ -0,0 +1,101 @@ +package ca.uhn.fhir.jpa.util; + +import java.util.ArrayList; +import java.util.List; + +/** + * Utility class for + */ +public class PermutationBuilder { + + /** + * Non instantiable + */ + private PermutationBuilder() { + // nothing + } + + /** + * Given a list of lists of options, returns a list of every possible combination of options. + * For example, given the input lists: + * 
+	 *   [
+	 *     [ A0, A1 ],
+	 *     [ B0, B1, B2 ]
+	 *   ]
+	 *
+ * This method will return the following output lists: + * 
+	 *   [
+	 *     [ A0, B0 ],
+	 *     [ A1, B0 ],
+	 *     [ A0, B1 ],
+	 *     [ A1, B1 ],
+	 *     [ A0, B2 ],
+	 *     [ A1, B2 ]
+	 *   ]
+	 *
+ *

+ * This method may or may not return a newly created list. + *

+ * + * @param theInput A list of lists to calculate permutations of + * @param The type associated with {@literal theInput}. The actual type doesn't matter, this method does not look at the + * values at all other than to copy them to the output lists. + * @since 7.4.0 + */ + public static List> calculatePermutations(List> theInput) { + if (theInput.size() == 1) { + return theInput; + } + List> listToPopulate = new ArrayList<>(calculatePermutationCount(theInput)); + int[] indices = new int[theInput.size()]; + doCalculatePermutationsIntoIndicesArrayAndPopulateList(0, indices, theInput, listToPopulate); + return listToPopulate; + } + + /** + * Recursively called/calling method which navigates across a list of input ArrayLists and populates the array of indices + * + * @param thePositionX The offset within {@literal theInput}. We navigate recursively from 0 through {@literal theInput.size()} + * @param theIndices The array to populate. This array has a length matching the size of {@literal theInput} and each entry + * represents an offset within the list at {@literal theInput.get(arrayIndex)} + * @param theInput The input List of ArrayLists + * @param theOutput A list to populate with all permutations + * @param The type associated with {@literal theInput}. The actual type doesn't matter, this method does not look at the + * values at all other than to copy them to the output lists. + */ + private static void doCalculatePermutationsIntoIndicesArrayAndPopulateList(int thePositionX, int[] theIndices, List> theInput, List> theOutput) { + if (thePositionX != theInput.size()) { + // If we're not at the end of the list of input vectors, recursively self-invoke once for each + // possible option at the current position in the list of input vectors. + for (int positionY = 0; positionY < theInput.get(thePositionX).size(); positionY++) { + theIndices[thePositionX] = positionY; + doCalculatePermutationsIntoIndicesArrayAndPopulateList(thePositionX + 1, theIndices, theInput, theOutput); + } + } else { + // If we're at the end of the list of input vectors, then we've been passed the + List nextList = new ArrayList<>(theInput.size()); + for (int positionX = 0; positionX < theInput.size(); positionX++) { + nextList.add(theInput.get(positionX).get(theIndices[positionX])); + } + theOutput.add(nextList); + } + } + + /** + * Returns the number of permutations that {@link #calculatePermutations(List)} will + * calculate for the given list. + * + * @throws ArithmeticException If the number of permutations exceeds {@link Integer#MAX_VALUE} + * @since 7.4.0 + */ + public static int calculatePermutationCount(List> theLists) throws ArithmeticException { + int retVal = !theLists.isEmpty() ? 1 : 0; + for (List theList : theLists) { + retVal = Math.multiplyExact(retVal, theList.size()); + } + return retVal; + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java new file mode 100644 index 00000000000..85016bb9bb7 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java @@ -0,0 +1,63 @@ +package ca.uhn.fhir.jpa.util; + +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.stream.IntStream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class PermutationBuilderTest { + + @Test + public void testCalculatePermutations() { + List> input = List.of( + List.of("A0", "A1"), + List.of("B0", "B1", "B2"), + List.of("C0", "C1") + ); + + List> output = PermutationBuilder.calculatePermutations(input); + assertThat(output).containsExactlyInAnyOrder( + List.of("A0", "B0", "C0"), + List.of("A1", "B0", "C0"), + List.of("A0", "B1", "C0"), + List.of("A1", "B1", "C0"), + List.of("A0", "B2", "C0"), + List.of("A1", "B2", "C0"), + List.of("A0", "B0", "C1"), + List.of("A1", "B0", "C1"), + List.of("A0", "B1", "C1"), + List.of("A1", "B1", "C1"), + List.of("A0", "B2", "C1"), + List.of("A1", "B2", "C1") + ); + } + + + @Test + public void testCalculatePermutationCount() { + List> input = List.of( + List.of("A0", "A1"), + List.of("B0", "B1", "B2"), + List.of("C0", "C1") + ); + assertEquals(12, PermutationBuilder.calculatePermutationCount(input)); + } + + @Test + public void testCalculatePermutationCountEmpty() { + List> input = List.of(); + assertEquals(0, PermutationBuilder.calculatePermutationCount(input)); + } + + @Test + public void testCalculatePermutationCountOverflow() { + List ranges = IntStream.range(0, 10001).boxed().toList(); + List> input = IntStream.range(0, 20).boxed().map(t -> ranges).toList(); + assertThrows(ArithmeticException.class, () -> PermutationBuilder.calculatePermutationCount(input)); + } + +} diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java index 2c00f66f3d2..10dd80e5c1b 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboNonUniqueParamTest.java @@ -7,21 +7,31 @@ import ca.uhn.fhir.jpa.searchparam.submit.interceptor.SearchParamValidatingInter import ca.uhn.fhir.jpa.util.SqlQuery; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.DateParam; +import ca.uhn.fhir.rest.param.ParamPrefixEnum; import ca.uhn.fhir.rest.param.ReferenceParam; +import ca.uhn.fhir.rest.param.StringAndListParam; import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.param.TokenAndListParam; +import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.util.HapiExtensions; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.BooleanType; +import org.hl7.fhir.r4.model.DateTimeType; import org.hl7.fhir.r4.model.DateType; import org.hl7.fhir.r4.model.Enumerations; import org.hl7.fhir.r4.model.Enumerations.PublicationStatus; +import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.SearchParameter; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import java.util.Comparator; @@ -106,8 +116,8 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T indexedTokens.sort(Comparator.comparing(ResourceIndexedComboTokenNonUnique::getIndexString)); assertEquals(4, indexedTokens.size()); String expected; - expected = "Patient?gender=http%3A%2F%2Ffoo1%7Cbar1&given=FAMILY1"; - assertEquals(expected, indexedTokens.get(0).getIndexString()); + expected = "Patient?gender=http%3A%2F%2Ffoo1%7Cbar1&given=FAMILY1"; + assertEquals(expected, indexedTokens.get(0).getIndexString()); expected = "Patient?gender=http%3A%2F%2Ffoo1%7Cbar1&given=FAMILY2"; assertEquals(expected, indexedTokens.get(1).getIndexString()); expected = "Patient?gender=http%3A%2F%2Ffoo2%7Cbar2&given=FAMILY1"; @@ -133,14 +143,14 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T List indexedTokens = myResourceIndexedComboTokensNonUniqueDao.findAll(); indexedTokens.sort(Comparator.comparing(ResourceIndexedComboTokenNonUnique::getId)); assertEquals(2, indexedTokens.size()); - String expected = "Patient?family=FAMILY1%5C%7C&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1"; + String expected = "Patient?family=FAMILY1&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1"; assertEquals(expected, indexedTokens.get(0).getIndexString()); - assertEquals(-7504889232313729794L, indexedTokens.get(0).getHashComplete().longValue()); + assertEquals(-2634469377090377342L, indexedTokens.get(0).getHashComplete().longValue()); }); myMessages.clear(); SearchParameterMap params = SearchParameterMap.newSynchronous(); - params.add("family", new StringParam("fAmIlY1|")); // weird casing to test normalization + params.add("family", new StringParam("fAmIlY1")); // weird casing to test normalization params.add("given", new StringParam("gIVEn1")); params.add("gender", new TokenParam("http://hl7.org/fhir/administrative-gender", "male")); myCaptureQueriesListener.clear(); @@ -149,12 +159,12 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue()); - assertThat(myCaptureQueriesListener.getSelectQueries().stream().map(t->t.getSql(true, false)).toList()).contains( - "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '-7504889232313729794')" + assertThat(myCaptureQueriesListener.getSelectQueries().stream().map(t -> t.getSql(true, false)).toList()).contains( + "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '-2634469377090377342')" ); logCapturedMessages(); - assertThat(myMessages.toString()).contains("[INFO Using NON_UNIQUE index for query for search: Patient?family=FAMILY1%5C%7C&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1]"); + assertThat(myMessages.toString()).contains("[INFO Using NON_UNIQUE index(es) for query for search: Patient?family=FAMILY1&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1]"); myMessages.clear(); // Remove 1, add another @@ -165,7 +175,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertNotNull(id3); params = SearchParameterMap.newSynchronous(); - params.add("family", new StringParam("fAmIlY1|")); // weird casing to test normalization + params.add("family", new StringParam("fAmIlY1")); // weird casing to test normalization params.add("given", new StringParam("gIVEn1")); params.add("gender", new TokenParam("http://hl7.org/fhir/administrative-gender", "male")); results = myPatientDao.search(params, mySrd); @@ -175,6 +185,23 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T } + @Test + public void testEmptyParamLists() { + createStringAndTokenCombo_NameAndGender(); + + IIdType id1 = createPatient1(null); + IIdType id2 = createPatient2(null); + + SearchParameterMap params = SearchParameterMap.newSynchronous(); + params.add("family", new StringAndListParam()); + params.add("given", new StringAndListParam()); + params.add("gender", new TokenAndListParam()); + IBundleProvider results = myPatientDao.search(params, mySrd); + List actual = toUnqualifiedVersionlessIdValues(results); + assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue(), id2.toUnqualifiedVersionless().getValue()); + + } + @Test public void testStringAndToken_CreateAndUpdate() { createStringAndTokenCombo_NameAndGender(); @@ -191,13 +218,13 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertEquals(1, myCaptureQueriesListener.countCommits()); assertEquals(0, myCaptureQueriesListener.countRollbacks()); - runInTransaction(()->{ + runInTransaction(() -> { List indexes = myResourceIndexedComboTokensNonUniqueDao .findAll() .stream() .map(ResourceIndexedComboTokenNonUnique::getIndexString) .toList(); - assertThat(indexes).as(indexes.toString()).containsExactly("Patient?family=FAMILY1%5C%7C&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1"); + assertThat(indexes).as(indexes.toString()).containsExactly("Patient?family=FAMILY1&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1"); }); /* @@ -217,13 +244,13 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertEquals(1, myCaptureQueriesListener.countCommits()); assertEquals(0, myCaptureQueriesListener.countRollbacks()); - runInTransaction(()->{ + runInTransaction(() -> { List indexes = myResourceIndexedComboTokensNonUniqueDao .findAll() .stream() .map(ResourceIndexedComboTokenNonUnique::getIndexString) .toList(); - assertThat(indexes).as(indexes.toString()).containsExactly("Patient?family=FAMILY2%5C%7C&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1"); + assertThat(indexes).as(indexes.toString()).containsExactly("Patient?family=FAMILY2&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1"); }); } @@ -243,12 +270,12 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T List indexedTokens = myResourceIndexedComboTokensNonUniqueDao.findAll(); indexedTokens.sort(Comparator.comparing(ResourceIndexedComboTokenNonUnique::getId)); assertEquals(2, indexedTokens.size()); - assertEquals(-7504889232313729794L, indexedTokens.get(0).getHashComplete().longValue()); + assertEquals(-2634469377090377342L, indexedTokens.get(0).getHashComplete().longValue()); }); myMessages.clear(); SearchParameterMap params = SearchParameterMap.newSynchronous(); - params.add("family", new StringParam("fAmIlY1|")); // weird casing to test normalization + params.add("family", new StringParam("fAmIlY1")); // weird casing to test normalization params.add("given", new StringParam("gIVEn1")); params.add("gender", new TokenParam("http://hl7.org/fhir/administrative-gender", "male")); params.add("birthdate", new DateParam("2021-02-02")); @@ -259,11 +286,43 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue()); String sql = myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false); - String expected = "SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_IDX_CMB_TOK_NU t0 ON (t1.RES_ID = t0.RES_ID) INNER JOIN HFJ_SPIDX_DATE t2 ON (t1.RES_ID = t2.RES_ID) WHERE ((t0.HASH_COMPLETE = '-7504889232313729794') AND ((t2.HASH_IDENTITY = '5247847184787287691') AND ((t2.SP_VALUE_LOW_DATE_ORDINAL >= '20210202') AND (t2.SP_VALUE_HIGH_DATE_ORDINAL <= '20210202'))))"; + String expected = "SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_IDX_CMB_TOK_NU t0 ON (t1.RES_ID = t0.RES_ID) INNER JOIN HFJ_SPIDX_DATE t2 ON (t1.RES_ID = t2.RES_ID) WHERE ((t0.HASH_COMPLETE = '-2634469377090377342') AND ((t2.HASH_IDENTITY = '5247847184787287691') AND ((t2.SP_VALUE_LOW_DATE_ORDINAL >= '20210202') AND (t2.SP_VALUE_HIGH_DATE_ORDINAL <= '20210202'))))"; assertEquals(expected, sql); logCapturedMessages(); - assertThat(myMessages.toString()).contains("[INFO Using NON_UNIQUE index for query for search: Patient?family=FAMILY1%5C%7C&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1]"); + assertThat(myMessages.toString()).contains("[INFO Using NON_UNIQUE index(es) for query for search: Patient?family=FAMILY1&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale&given=GIVEN1]"); + myMessages.clear(); + + } + + + @Test + public void testStringAndToken_MultipleAnd() { + createStringAndTokenCombo_NameAndGender(); + + IIdType id1 = createPatient(withFamily("SIMPSON"), withGiven("HOMER"), withGiven("JAY"), withGender("male")); + assertNotNull(id1); + + logAllNonUniqueIndexes(); + logAllStringIndexes(); + + myMessages.clear(); + SearchParameterMap params = SearchParameterMap.newSynchronous(); + params.add("family", new StringParam("Simpson")); + params.add("given", new StringAndListParam().addAnd(new StringParam("Homer")).addAnd(new StringParam("Jay"))); + params.add("gender", new TokenParam("male")); + myCaptureQueriesListener.clear(); + IBundleProvider results = myPatientDao.search(params, mySrd); + List actual = toUnqualifiedVersionlessIdValues(results); + myCaptureQueriesListener.logSelectQueries(); + assertThat(actual).containsExactlyInAnyOrder(id1.toUnqualifiedVersionless().getValue()); + + String sql = myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false); + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_SPIDX_STRING t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '7545664593829342272') AND ((t1.HASH_NORM_PREFIX = '6206712800146298788') AND (t1.SP_VALUE_NORMALIZED LIKE 'JAY%')))"; + assertEquals(expected, sql); + + logCapturedMessages(); + assertThat(myMessages.toString()).contains("Using NON_UNIQUE index(es) for query for search: Patient?family=SIMPSON&gender=male&given=HOMER"); myMessages.clear(); } @@ -271,7 +330,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T @Test public void testStringAndDate_Create() { - createStringAndTokenCombo_NameAndBirthdate(); + createStringAndDateCombo_NameAndBirthdate(); IIdType id1 = createPatient1(null); assertNotNull(id1); @@ -303,45 +362,10 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); logCapturedMessages(); - assertThat(myMessages.toString()).contains("[INFO Using NON_UNIQUE index for query for search: Patient?birthdate=2021-02-02&family=FAMILY1]"); + assertThat(myMessages.toString()).contains("[INFO Using NON_UNIQUE index(es) for query for search: Patient?birthdate=2021-02-02&family=FAMILY1]"); myMessages.clear(); } - /** - * Can't create or search for combo params with partial dates - */ - @Test - public void testStringAndDate_Create_PartialDate() { - createStringAndTokenCombo_NameAndBirthdate(); - - Patient pt1 = new Patient(); - pt1.getNameFirstRep().setFamily("Family1").addGiven("Given1"); - pt1.setBirthDateElement(new DateType("2021-02")); - IIdType id1 = myPatientDao.create(pt1, mySrd).getId().toUnqualified(); - - logAllNonUniqueIndexes(); - runInTransaction(() -> { - List indexedTokens = myResourceIndexedComboTokensNonUniqueDao.findAll(); - assertEquals(0, indexedTokens.size()); - }); - - myMessages.clear(); - SearchParameterMap params = SearchParameterMap.newSynchronous(); - params.add("family", new StringParam("family1")); - params.add("birthdate", new DateParam("2021-02")); - myCaptureQueriesListener.clear(); - IBundleProvider results = myPatientDao.search(params, mySrd); - List actual = toUnqualifiedVersionlessIdValues(results); - myCaptureQueriesListener.logSelectQueries(); - assertThat(actual).contains(id1.toUnqualifiedVersionless().getValue()); - - String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertThat(sql).doesNotContain("HFJ_IDX_CMB_TOK_NU"); - - logCapturedMessages(); - assertThat(myMessages).isEmpty(); - } - @Test public void testStringAndReference_Create() { createStringAndReferenceCombo_FamilyAndOrganization(); @@ -356,12 +380,12 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T List indexedTokens = myResourceIndexedComboTokensNonUniqueDao.findAll(); indexedTokens.sort(Comparator.comparing(ResourceIndexedComboTokenNonUnique::getId)); assertEquals(2, indexedTokens.size()); - assertEquals("Patient?family=FAMILY1%5C%7C&organization=Organization%2Fmy-org", indexedTokens.get(0).getIndexString()); + assertEquals("Patient?family=FAMILY1&organization=Organization%2Fmy-org", indexedTokens.get(0).getIndexString()); }); myMessages.clear(); SearchParameterMap params = SearchParameterMap.newSynchronous(); - params.add("family", new StringParam("fAmIlY1|")); // weird casing to test normalization + params.add("family", new StringParam("fAmIlY1")); // weird casing to test normalization params.add("organization", new ReferenceParam(ORG_ID_QUALIFIED)); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.search(params, mySrd); @@ -369,7 +393,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); assertThat(actual).contains(id1.toUnqualifiedVersionless().getValue()); - String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '2277801301223576208')"; + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE = '2591238402961312979')"; assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); } @@ -394,8 +418,8 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myCaptureQueriesListener.logSelectQueries(); String expected; - expected = "select rt1_0.RES_ID,rt1_0.RES_TYPE,rt1_0.FHIR_ID from HFJ_RESOURCE rt1_0 where rt1_0.FHIR_ID='my-org'"; - assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); + expected = "select rt1_0.RES_ID,rt1_0.RES_TYPE,rt1_0.FHIR_ID from HFJ_RESOURCE rt1_0 where rt1_0.FHIR_ID='my-org'"; + assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(1).getSql(true, false); assertThat(sql).contains("SP_VALUE_NORMALIZED LIKE 'FAMILY1%'"); assertThat(sql).contains("t1.TARGET_RESOURCE_ID"); @@ -403,6 +427,33 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T assertThat(myMessages.get(0)).contains("This search uses an unqualified resource"); } + @Test + public void testOrQuery() { + createTokenAndReferenceCombo_FamilyAndOrganization(); + + createPatient(withId("PAT"), withActiveTrue()); + createObservation(withId("O1"), withSubject("Patient/PAT"), withStatus("final")); + createObservation(withId("O2"), withSubject("Patient/PAT"), withStatus("registered")); + + SearchParameterMap params = SearchParameterMap.newSynchronous(); + params.add("patient", new ReferenceParam("Patient/PAT")); + params.add("status", new TokenOrListParam(null, "preliminary", "final", "amended")); + myCaptureQueriesListener.clear(); + IBundleProvider results = myObservationDao.search(params, mySrd); + List actual = toUnqualifiedVersionlessIdValues(results); + myCaptureQueriesListener.logSelectQueries(); + assertThat(actual).contains("Observation/O1"); + + String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 WHERE (t0.HASH_COMPLETE IN ('2445648980345828396','-6884698528022589694','-8034948665712960724') )"; + assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false)); + + logCapturedMessages(); + assertThat(myMessages.toString()).contains("Observation?patient=Patient%2FPAT&status=amended", "Observation?patient=Patient%2FPAT&status=final", "Observation?patient=Patient%2FPAT&status=preliminary"); + myMessages.clear(); + + } + + private void createOrg() { Organization org = new Organization(); org.setName("Some Org"); @@ -410,7 +461,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myOrganizationDao.update(org, mySrd); } - private void createStringAndTokenCombo_NameAndBirthdate() { + private void createStringAndDateCombo_NameAndBirthdate() { SearchParameter sp = new SearchParameter(); sp.setId("SearchParameter/patient-family"); sp.setType(Enumerations.SearchParamType.STRING); @@ -455,7 +506,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T sp.setId("SearchParameter/patient-family"); sp.setType(Enumerations.SearchParamType.STRING); sp.setCode("family"); - sp.setExpression("Patient.name.family + '|'"); + sp.setExpression("Patient.name.family"); sp.setStatus(PublicationStatus.ACTIVE); sp.addBase("Patient"); mySearchParameterDao.update(sp, mySrd); @@ -507,7 +558,7 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T sp.setId("SearchParameter/patient-family"); sp.setType(Enumerations.SearchParamType.STRING); sp.setCode("family"); - sp.setExpression("Patient.name.family + '|'"); + sp.setExpression("Patient.name.family"); sp.setStatus(PublicationStatus.ACTIVE); sp.addBase("Patient"); mySearchParameterDao.update(sp, mySrd); @@ -542,6 +593,45 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T myMessages.clear(); } + private void createTokenAndReferenceCombo_FamilyAndOrganization() { + SearchParameter sp = new SearchParameter(); + sp.setId("SearchParameter/Observation-status"); + sp.setType(Enumerations.SearchParamType.TOKEN); + sp.setCode(Observation.SP_STATUS); + sp.setExpression("Observation.status"); + sp.setStatus(PublicationStatus.ACTIVE); + sp.addBase("Observation"); + mySearchParameterDao.update(sp, mySrd); + + sp = new SearchParameter(); + sp.setId("SearchParameter/clinical-patient"); + sp.setType(Enumerations.SearchParamType.REFERENCE); + sp.setCode(Observation.SP_PATIENT); + sp.setExpression("Observation.subject.where(resolve() is Patient)"); + sp.setStatus(PublicationStatus.ACTIVE); + sp.addBase("Observation"); + mySearchParameterDao.update(sp, mySrd); + + sp = new SearchParameter(); + sp.setId("SearchParameter/observation-status-and-patient"); + sp.setType(Enumerations.SearchParamType.COMPOSITE); + sp.setStatus(PublicationStatus.ACTIVE); + sp.addBase("Observation"); + sp.addComponent() + .setExpression("Observation") + .setDefinition("SearchParameter/Observation-status"); + sp.addComponent() + .setExpression("Observation") + .setDefinition("SearchParameter/clinical-patient"); + sp.addExtension() + .setUrl(HapiExtensions.EXT_SP_UNIQUE) + .setValue(new BooleanType(false)); + mySearchParameterDao.update(sp, mySrd); + + mySearchParamRegistry.forceRefresh(); + + myMessages.clear(); + } private IIdType createPatient1(String theOrgId) { Patient pt1 = new Patient(); @@ -561,5 +651,143 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T return myPatientDao.create(pt2, mySrd).getId().toUnqualified(); } + /** + * A few tests where the combo index should not be used + */ + @Nested + public class ShouldNotUseComboIndexTest { + + @BeforeEach + public void before() { + SearchParameter sp = new SearchParameter(); + sp.setId("SearchParameter/Observation-date"); + sp.setType(Enumerations.SearchParamType.DATE); + sp.setCode("date"); + sp.setExpression("Observation.effective"); + sp.setStatus(PublicationStatus.ACTIVE); + sp.addBase("Observation"); + mySearchParameterDao.update(sp, mySrd); + + sp = new SearchParameter(); + sp.setId("SearchParameter/Observation-note-text"); + sp.setType(Enumerations.SearchParamType.STRING); + sp.setCode("note-text"); + sp.setExpression("Observation.note.text"); + sp.setStatus(PublicationStatus.ACTIVE); + sp.addBase("Observation"); + mySearchParameterDao.update(sp, mySrd); + + sp = new SearchParameter(); + sp.setId("SearchParameter/observation-date-and-note-text"); + sp.setType(Enumerations.SearchParamType.COMPOSITE); + sp.setStatus(PublicationStatus.ACTIVE); + sp.addBase("Observation"); + sp.addComponent() + .setExpression("Observation") + .setDefinition("SearchParameter/Observation-date"); + sp.addComponent() + .setExpression("Observation") + .setDefinition("SearchParameter/Observation-note-text"); + sp.addExtension() + .setUrl(HapiExtensions.EXT_SP_UNIQUE) + .setValue(new BooleanType(false)); + mySearchParameterDao.update(sp, mySrd); + + mySearchParamRegistry.forceRefresh(); + + myMessages.clear(); + } + + /** + * Can't create or search for combo params with dateTimes that don't have DAY precision + */ + @ParameterizedTest + @CsvSource(value = { + "2021-01-02T12:00:01.000Z , false", + "2021-01-02T12:00:01Z , false", + "2021-01-02 , true", // <-- DAY precision + "2021-01 , false", + "2021 , false", + }) + public void testPartialDateTime(String theDateValue, boolean theShouldUseComboIndex) { + IIdType id1 = createObservation(theDateValue); + + logAllNonUniqueIndexes(); + runInTransaction(() -> { + List indexedTokens = myResourceIndexedComboTokensNonUniqueDao.findAll(); + if (theShouldUseComboIndex) { + assertEquals(1, indexedTokens.size()); + } else { + assertEquals(0, indexedTokens.size()); + } + }); + + SearchParameterMap params = SearchParameterMap + .newSynchronous() + .add("note-text", new StringParam("Hello")) + .add("date", new DateParam(theDateValue)); + myCaptureQueriesListener.clear(); + IBundleProvider results = myObservationDao.search(params, mySrd); + List actual = toUnqualifiedVersionlessIdValues(results); + myCaptureQueriesListener.logSelectQueries(); + assertThat(actual).contains(id1.toUnqualifiedVersionless().getValue()); + + if (theShouldUseComboIndex) { + assertComboIndexUsed(); + } else { + assertComboIndexNotUsed(); + assertThat(myMessages.toString()).contains("INFO Search with params [date, note-text] is not a candidate for combo searching - Date search with non-DAY precision for parameter 'date'"); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testDateModifier(boolean theUseComparator) { + IIdType id1 = createObservation("2021-01-02"); + createObservation("2023-01-02"); + + SearchParameterMap params = SearchParameterMap + .newSynchronous() + .add("note-text", new StringParam("Hello")) + .add("date", new DateParam("2021-01-02").setPrefix(theUseComparator ? ParamPrefixEnum.LESSTHAN_OR_EQUALS : null)); + myCaptureQueriesListener.clear(); + IBundleProvider results = myObservationDao.search(params, mySrd); + List actual = toUnqualifiedVersionlessIdValues(results); + myCaptureQueriesListener.logSelectQueries(); + assertThat(actual).contains(id1.toUnqualifiedVersionless().getValue()); + + if (theUseComparator) { + assertComboIndexNotUsed(); + assertThat(myMessages.toString()).contains("INFO Search with params [date, note-text] is not a candidate for combo searching - Parameter 'date' has prefix: 'le'"); + } else { + assertComboIndexUsed(); + } + + } + + private void assertComboIndexUsed() { + String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + assertThat(sql).contains("HFJ_IDX_CMB_TOK_NU"); + + assertThat(myMessages.toString()).contains("Using NON_UNIQUE index(es) for query"); + } + + private void assertComboIndexNotUsed() { + String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + assertThat(sql).doesNotContain("HFJ_IDX_CMB_TOK_NU"); + + assertThat(myMessages.toString()).doesNotContain("Using NON_UNIQUE index(es) for query"); + } + + private IIdType createObservation(String theDateValue) { + Observation pt1 = new Observation(); + pt1.addNote().setText("Hello"); + pt1.setEffective(new DateTimeType(theDateValue)); + IIdType id1 = myObservationDao.create(pt1, mySrd).getId().toUnqualified(); + return id1; + } + + } + } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java index 507ac392a77..7c3726df589 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java @@ -13,6 +13,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.util.JpaParamUtil; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.param.DateOrListParam; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.TokenAndListParam; @@ -471,10 +472,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1); unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertThat(unformattedSql).contains("HASH_SYS_AND_VALUE IN ('4101160957635429999','-3122824860083758210')"); - assertThat(unformattedSql).doesNotContain(("IDX_STRING")); - assertThat(unformattedSql).doesNotContain(("RES_DELETED_AT")); - assertThat(unformattedSql).doesNotContain(("RES_TYPE")); + assertEquals("SELECT t0.RES_ID FROM HFJ_IDX_CMP_STRING_UNIQ t0 WHERE (t0.IDX_STRING IN ('Patient?identifier=urn%7C111','Patient?identifier=urn%7C222') )", unformattedSql); // Not matching the composite SP at all myCaptureQueriesListener.clear(); @@ -1110,6 +1108,40 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { } + @Test + public void testOrQuery() { + myStorageSettings.setAdvancedHSearchIndexing(false); + createUniqueBirthdateAndGenderSps(); + + Patient pt1 = new Patient(); + pt1.setGender(Enumerations.AdministrativeGender.MALE); + pt1.setBirthDateElement(new DateType("2011-01-01")); + IIdType id1 = myPatientDao.create(pt1, mySrd).getId().toUnqualifiedVersionless(); + + Patient pt2 = new Patient(); + pt2.setGender(Enumerations.AdministrativeGender.MALE); + pt2.setBirthDateElement(new DateType("2011-01-02")); + IIdType id2 = myPatientDao.create(pt2, mySrd).getId().toUnqualifiedVersionless(); + + myCaptureQueriesListener.clear(); + myMessages.clear(); + SearchParameterMap params = new SearchParameterMap(); + params.setLoadSynchronousUpTo(100); + params.add("gender", new TokenParam("http://hl7.org/fhir/administrative-gender", "male")); + params.add("birthdate", new DateOrListParam().addOr(new DateParam("2011-01-01")).addOr(new DateParam("2011-01-02"))); + myCaptureQueriesListener.clear(); + IBundleProvider results = myPatientDao.search(params, mySrd); + myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(results)).containsExactlyInAnyOrder(id1.getValue(), id2.getValue()); + + assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false)) + .contains("SELECT t0.RES_ID FROM HFJ_IDX_CMP_STRING_UNIQ t0 WHERE (t0.IDX_STRING IN ('Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale','Patient?birthdate=2011-01-02&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale') )"); + logCapturedMessages(); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: [Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale, Patient?birthdate=2011-01-02&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale]"); + myMessages.clear(); + + } + @Test public void testSearchSynchronousUsingUniqueComposite() { myStorageSettings.setAdvancedHSearchIndexing(false); @@ -1136,7 +1168,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { assertThat(toUnqualifiedVersionlessIdValues(results)).containsExactlyInAnyOrder(id1.getValue()); logCapturedMessages(); - assertThat(myMessages.toString()).contains("Using UNIQUE index for query for search: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); myMessages.clear(); } @@ -1164,7 +1196,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { String searchId = results.getUuid(); assertThat(toUnqualifiedVersionlessIdValues(results)).containsExactlyInAnyOrder(id1); logCapturedMessages(); - assertThat(myMessages.toString()).contains("Using UNIQUE index for query for search: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); myMessages.clear(); // Other order @@ -1188,7 +1220,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { results = myPatientDao.search(params, mySrd); assertThat(toUnqualifiedVersionlessIdValues(results)).isEmpty(); logCapturedMessages(); - assertThat(myMessages.toString()).contains("Using UNIQUE index for query for search: Patient?birthdate=2011-01-03&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: Patient?birthdate=2011-01-03&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); myMessages.clear(); myMessages.clear(); @@ -1270,7 +1302,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { IIdType id1 = myPatientDao.update(pt1, "Patient?name=FAMILY1&organization:Organization=ORG", mySrd).getId().toUnqualifiedVersionless(); logCapturedMessages(); - assertThat(myMessages.toString()).contains("Using UNIQUE index for query for search: Patient?name=FAMILY1&organization=Organization%2FORG"); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: Patient?name=FAMILY1&organization=Organization%2FORG"); myMessages.clear(); runInTransaction(() -> { @@ -1289,7 +1321,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { IIdType id2 = myPatientDao.update(pt1, "Patient?name=FAMILY1&organization:Organization=ORG", mySrd).getId().toUnqualifiedVersionless(); logCapturedMessages(); - assertThat(myMessages.toString()).contains("Using UNIQUE index for query for search: Patient?name=FAMILY1&organization=Organization%2FORG"); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: Patient?name=FAMILY1&organization=Organization%2FORG"); myMessages.clear(); runInTransaction(() -> { List uniques = myResourceIndexedComboStringUniqueDao.findAll(); @@ -1646,7 +1678,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { assertThat(toUnqualifiedVersionlessIdValues(results)).containsExactlyInAnyOrder(id2.getValue()); logCapturedMessages(); - assertThat(myMessages.toString()).contains("Using UNIQUE index for query for search: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); + assertThat(myMessages.toString()).contains("Using UNIQUE index(es) for query for search: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"); myMessages.clear(); }