Issue with combo index search parameter when using date

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java

@@ -28,6 +28,7 @@
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.util.DateUtils;
 import jakarta.annotation.Nonnull;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.Validate;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 
@@ -182,11 +183,9 @@ public DateRangeParam(String theLowerBound, String theUpperBound) {
 	}
 
 	private void addParam(DateParam theParsed) throws InvalidRequestException {
-		if (theParsed.getPrefix() == null) {
-			theParsed.setPrefix(EQUAL);
-		}
+		ParamPrefixEnum prefix = getPrefixOrDefault(theParsed);
 
-		switch (theParsed.getPrefix()) {
+		switch (prefix) {
 			case NOT_EQUAL:
 			case EQUAL:
 				if (myLowerBound != null || myUpperBound != null) {
@@ -312,24 +311,24 @@ public Integer getLowerBoundAsDateInteger() {
 		}
 		int retVal = DateUtils.convertDateToDayInteger(myLowerBound.getValue());
 
-		if (myLowerBound.getPrefix() != null) {
-			switch (myLowerBound.getPrefix()) {
-				case GREATERTHAN:
-				case STARTS_AFTER:
-					retVal += 1;
-					break;
-				case EQUAL:
-				case GREATERTHAN_OR_EQUALS:
-				case NOT_EQUAL:
-					break;
-				case LESSTHAN:
-				case LESSTHAN_OR_EQUALS:
-				case APPROXIMATE:
-				case ENDS_BEFORE:
-					throw new IllegalStateException(
-							Msg.code(1926) + "Invalid lower bound comparator: " + myLowerBound.getPrefix());
-			}
+		ParamPrefixEnum prefix = getPrefixOrDefault(myLowerBound);
+
+		switch (prefix) {
+			case GREATERTHAN:
+			case STARTS_AFTER:
+				retVal += 1;
+				break;
+			case EQUAL:
+			case GREATERTHAN_OR_EQUALS:
+			case NOT_EQUAL:
+				break;
+			case LESSTHAN:
+			case LESSTHAN_OR_EQUALS:
+			case APPROXIMATE:
+			case ENDS_BEFORE:
+				throw new IllegalStateException(Msg.code(1926) + "Invalid lower bound comparator: " + prefix);
 		}
+
 		return retVal;
 	}
 
@@ -343,24 +342,25 @@ public Integer getUpperBoundAsDateInteger() {
 			return null;
 		}
 		int retVal = DateUtils.convertDateToDayInteger(myUpperBound.getValue());
-		if (myUpperBound.getPrefix() != null) {
-			switch (myUpperBound.getPrefix()) {
-				case LESSTHAN:
-				case ENDS_BEFORE:
-					retVal -= 1;
-					break;
-				case EQUAL:
-				case LESSTHAN_OR_EQUALS:
-				case NOT_EQUAL:
-					break;
-				case GREATERTHAN_OR_EQUALS:
-				case GREATERTHAN:
-				case APPROXIMATE:
-				case STARTS_AFTER:
-					throw new IllegalStateException(
-							Msg.code(1927) + "Invalid upper bound comparator: " + myUpperBound.getPrefix());
-			}
+
+		ParamPrefixEnum prefix = getPrefixOrDefault(myUpperBound);
+
+		switch (prefix) {
+			case LESSTHAN:
+			case ENDS_BEFORE:
+				retVal -= 1;
+				break;
+			case EQUAL:
+			case LESSTHAN_OR_EQUALS:
+			case NOT_EQUAL:
+				break;
+			case GREATERTHAN_OR_EQUALS:
+			case GREATERTHAN:
+			case APPROXIMATE:
+			case STARTS_AFTER:
+				throw new IllegalStateException(Msg.code(1927) + "Invalid upper bound comparator: " + prefix);
 		}
+
 		return retVal;
 	}
 
@@ -374,28 +374,29 @@ public Date getLowerBoundAsInstant() {
 	@Nonnull
 	private static Date getLowerBoundAsInstant(@Nonnull DateParam theLowerBound) {
 		Date retVal = theLowerBound.getValue();
+
 		if (theLowerBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) {
 			retVal = DateUtils.getLowestInstantFromDate(retVal);
 		}
 
-		if (theLowerBound.getPrefix() != null) {
-			switch (theLowerBound.getPrefix()) {
-				case GREATERTHAN:
-				case STARTS_AFTER:
-					retVal = theLowerBound.getPrecision().add(retVal, 1);
-					break;
-				case EQUAL:
-				case NOT_EQUAL:
-				case GREATERTHAN_OR_EQUALS:
-					break;
-				case LESSTHAN_OR_EQUALS:
-				case LESSTHAN:
-				case APPROXIMATE:
-				case ENDS_BEFORE:
-					throw new IllegalStateException(
-							Msg.code(1928) + "Invalid lower bound comparator: " + theLowerBound.getPrefix());
-			}
+		ParamPrefixEnum prefix = getPrefixOrDefault(theLowerBound);
+
+		switch (prefix) {
+			case GREATERTHAN:
+			case STARTS_AFTER:
+				retVal = theLowerBound.getPrecision().add(retVal, 1);
+				break;
+			case EQUAL:
+			case NOT_EQUAL:
+			case GREATERTHAN_OR_EQUALS:
+				break;
+			case LESSTHAN_OR_EQUALS:
+			case LESSTHAN:
+			case APPROXIMATE:
+			case ENDS_BEFORE:
+				throw new IllegalStateException(Msg.code(1928) + "Invalid lower bound comparator: " + prefix);
 		}
+
 		return retVal;
 	}
 
@@ -446,26 +447,26 @@ private static Date getUpperBoundAsInstant(@Nonnull DateParam theUpperBound) {
 			retVal = DateUtils.getHighestInstantFromDate(retVal);
 		}
 
-		if (theUpperBound.getPrefix() != null) {
-			switch (theUpperBound.getPrefix()) {
-				case LESSTHAN:
-				case ENDS_BEFORE:
-					retVal = new Date(retVal.getTime() - 1L);
-					break;
-				case EQUAL:
-				case NOT_EQUAL:
-				case LESSTHAN_OR_EQUALS:
-					retVal = theUpperBound.getPrecision().add(retVal, 1);
-					retVal = new Date(retVal.getTime() - 1L);
-					break;
-				case GREATERTHAN_OR_EQUALS:
-				case GREATERTHAN:
-				case APPROXIMATE:
-				case STARTS_AFTER:
-					throw new IllegalStateException(
-							Msg.code(1929) + "Invalid upper bound comparator: " + theUpperBound.getPrefix());
-			}
+		ParamPrefixEnum prefix = getPrefixOrDefault(theUpperBound);
+
+		switch (prefix) {
+			case LESSTHAN:
+			case ENDS_BEFORE:
+				retVal = new Date(retVal.getTime() - 1L);
+				break;
+			case EQUAL:
+			case NOT_EQUAL:
+			case LESSTHAN_OR_EQUALS:
+				retVal = theUpperBound.getPrecision().add(retVal, 1);
+				retVal = new Date(retVal.getTime() - 1L);
+				break;
+			case GREATERTHAN_OR_EQUALS:
+			case GREATERTHAN:
+			case APPROXIMATE:
+			case STARTS_AFTER:
+				throw new IllegalStateException(Msg.code(1929) + "Invalid upper bound comparator: " + prefix);
 		}
+
 		return retVal;
 	}
 
@@ -586,6 +587,16 @@ public void setValuesAsQueryTokens(
 			FhirContext theContext, String theParamName, List<QualifiedParamList> theParameters)
 			throws InvalidRequestException {
 
+		// When we create and populate a DateRangeParam from a query parameter (?birthdate=2024-12-02 or
+		// ?birthdate=eq2024-12-02), we
+		// set the prefix only if it was specifically provided by the client as it is mandatory to retain the capability
+		// to make the differentiation. See {@link SearchBuilder#validateParamValuesAreValidForComboParam}.
+		//
+		// Since the FHIR specification says that "If no prefix is present, the prefix <code>eq</code> is assumed",
+		// we will do so by invoking method {@link DateRangeParam#getPrefixOrDefault} everytime computation is
+		// conditional on the
+		// prefix value.
+
 		boolean haveHadUnqualifiedParameter = false;
 		for (QualifiedParamList paramList : theParameters) {
 			if (paramList.size() == 0) {
@@ -701,4 +712,13 @@ private void validateAndSet(DateParam lowerBound, DateParam upperBound) {
 		myLowerBound = lowerBound;
 		myUpperBound = upperBound;
 	}
+
+	/**
+	 *
+	 * This method should be used when performing computation conditional on the prefix value to ensure that a dateParam
+	 * without prefix is treated as if it has one set to 'eq'.
+	 */
+	private static ParamPrefixEnum getPrefixOrDefault(DateParam theDateParam) {
+		return ObjectUtils.defaultIfNull(theDateParam.getPrefix(), EQUAL);
+	}
 }

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6603-combo-index-sp-not-invoked-with-dateparam.yaml

@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 6603
+jira: SMILE-8048
+title: "Previously, searches where processing was optimised with a combo index search parameters would skip searching the 
+optimised indexes when the query string included a date range (i.e. multiple instances of the same date parameter). This
+issue is fixed."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java

@@ -826,10 +826,7 @@ private boolean isNotEqualsComparator(DateRangeParam theDateRange) {
 			DateParam lb = theDateRange.getLowerBound();
 			DateParam ub = theDateRange.getUpperBound();
 
-			return lb != null
-					&& ub != null
-					&& lb.getPrefix().equals(NOT_EQUAL)
-					&& ub.getPrefix().equals(NOT_EQUAL);
+			return lb != null && ub != null && NOT_EQUAL.equals(lb.getPrefix()) && NOT_EQUAL.equals(ub.getPrefix());
 		}
 		return false;
 	}

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java

@@ -567,8 +567,8 @@ public String toNormalizedQueryString(FhirContext theCtx) {
 	private boolean isNotEqualsComparator(DateParam theLowerBound, DateParam theUpperBound) {
 		return theLowerBound != null
 				&& theUpperBound != null
-				&& theLowerBound.getPrefix().equals(NOT_EQUAL)
-				&& theUpperBound.getPrefix().equals(NOT_EQUAL);
+				&& NOT_EQUAL.equals(theLowerBound.getPrefix())
+				&& NOT_EQUAL.equals(theUpperBound.getPrefix());
 	}
 
 	/**

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseComboParamsR4Test.java

@@ -7,6 +7,7 @@
 import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
 import ca.uhn.fhir.jpa.search.reindex.ResourceReindexingSvcImpl;
 import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
+import ca.uhn.fhir.jpa.test.util.ComboSearchParameterTestHelper;
 import ca.uhn.fhir.jpa.util.SpringObjectCaster;
 import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
 import ca.uhn.fhir.test.utilities.MockInvoker;
@@ -30,6 +31,7 @@ public abstract class BaseComboParamsR4Test extends BaseJpaR4Test {
 	protected ISearchParamRegistry mySearchParamRegistry;
 	protected List<String> myMessages = new ArrayList<>();
 	private IInterceptorBroadcaster myInterceptorBroadcaster;
+	protected ComboSearchParameterTestHelper myComboSearchParameterTestHelper;
 
 	@Override
 	@BeforeEach
@@ -62,6 +64,7 @@ public void before() throws Exception {
 
 		// allow searches to use cached results
 		when(myInterceptorBroadcaster.getInvokersForPointcut(eq(Pointcut.STORAGE_PRECHECK_FOR_CACHED_SEARCH))).thenReturn(MockInvoker.list(params->true));
+		myComboSearchParameterTestHelper = new ComboSearchParameterTestHelper(mySearchParameterDao, mySearchParamRegistry);
 	}
 
 	@AfterEach
@@ -80,5 +83,9 @@ protected void logCapturedMessages() {
 		ourLog.info("Messages:\n  {}", String.join("\n  ", myMessages));
 	}
 
+	protected void createBirthdateAndGenderSps(boolean theUnique) {
+		myComboSearchParameterTestHelper.createBirthdateAndGenderSps(theUnique);
+		myMessages.clear();
+	}
 
 }