partial merge of SearchBuilder changes

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6261-upgrade-to-jakarta-mail.yaml

@@ -0,0 +1,5 @@
+---
+type: change
+issue: 6261
+title: "Upgrading to Jakarta had caused a problem with the version of java-simple-mail that was in use. This has been updated to be conformant
+with the Jakarta Mail APIs. Thanks to Thomas Papke(@thopap) for the contribution!"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6290-mssql-migration-trim-fix.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6290
+title: "Previously, a specific migration task was using the `TRIM()` function, which does not exist in MSSQL 2012. This was causing migrations targeting MSSQL 2012 to fail. 
+This has been corrected and replaced with usage of a combination of LTRIM() and RTRIM(). Thanks to Primož Delopst at Better for the contribution!"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6317-fix-resource-duplication-for-composite-unique-sp-with-date-time-component.yaml

@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 6317
+title: "Previously, defining a unique combo Search Parameter with the DateTime component and submitting multiple 
+resources with the same dateTime element (e.g. Observation.effectiveDateTime) resulted in duplicate resource creation. 
+This has been fixed."
+

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6325-partitioned-search.yaml

@@ -0,0 +1,4 @@
+---
+type: add
+issue: 6325
+title: "Add support for including the partitioning column in search query joins."

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

@@ -361,7 +361,7 @@ public void addSortOnResourceLink(
 			case STRING:
 				StringPredicateBuilder stringPredicateBuilder = mySqlBuilder.createStringPredicateBuilder();
 				addSortCustomJoin(
-						resourceLinkPredicateBuilder.getColumnTargetResourceId(),
+						resourceLinkPredicateBuilder.getJoinColumnsForTarget(),
 						stringPredicateBuilder,
 						stringPredicateBuilder.createHashIdentityPredicate(targetType, theChain));
 
@@ -372,7 +372,7 @@ public void addSortOnResourceLink(
 			case TOKEN:
 				TokenPredicateBuilder tokenPredicateBuilder = mySqlBuilder.createTokenPredicateBuilder();
 				addSortCustomJoin(
-						resourceLinkPredicateBuilder.getColumnTargetResourceId(),
+						resourceLinkPredicateBuilder.getJoinColumnsForTarget(),
 						tokenPredicateBuilder,
 						tokenPredicateBuilder.createHashIdentityPredicate(targetType, theChain));
 
@@ -383,7 +383,7 @@ public void addSortOnResourceLink(
 			case DATE:
 				DatePredicateBuilder datePredicateBuilder = mySqlBuilder.createDatePredicateBuilder();
 				addSortCustomJoin(
-						resourceLinkPredicateBuilder.getColumnTargetResourceId(),
+						resourceLinkPredicateBuilder.getJoinColumnsForTarget(),
 						datePredicateBuilder,
 						datePredicateBuilder.createHashIdentityPredicate(targetType, theChain));
 
@@ -477,24 +477,24 @@ private void addSortCustomJoin(
 			BaseJoiningPredicateBuilder theFromJoiningPredicateBuilder,
 			BaseJoiningPredicateBuilder theToJoiningPredicateBuilder,
 			Condition theCondition) {
-		addSortCustomJoin(
-				theFromJoiningPredicateBuilder.getResourceIdColumn(), theToJoiningPredicateBuilder, theCondition);
+		addSortCustomJoin(theFromJoiningPredicateBuilder.getJoinColumns(), theToJoiningPredicateBuilder, theCondition);
 	}
 
 	private void addSortCustomJoin(
-			DbColumn theFromDbColumn,
+			DbColumn theFromDbColumn[],
 			BaseJoiningPredicateBuilder theToJoiningPredicateBuilder,
 			Condition theCondition) {
+
 		ComboCondition onCondition =
-				mySqlBuilder.createOnCondition(theFromDbColumn, theToJoiningPredicateBuilder.getResourceIdColumn());
+				mySqlBuilder.createOnCondition(theFromDbColumn, theToJoiningPredicateBuilder.getJoinColumns());
 
 		if (theCondition != null) {
 			onCondition.addCondition(theCondition);
 		}
 
 		mySqlBuilder.addCustomJoin(
 				SelectQuery.JoinType.LEFT_OUTER,
-				theFromDbColumn.getTable(),
+				theFromDbColumn[0].getTable(),
 				theToJoiningPredicateBuilder.getTable(),
 				onCondition);
 	}
@@ -1482,7 +1482,11 @@ public Condition createPredicateReference(
 
 	public void addGrouping() {
 		BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
-		mySqlBuilder.getSelect().addGroupings(firstPredicateBuilder.getJoinColumns());
+		// mySqlBuilder.getSelect().addGroupings(firstPredicateBuilder.getJoinColumns());
+		// we always select partition_id, even if not joining.
+		mySqlBuilder.getSelect().addGroupings(new DbColumn[] {
+			firstPredicateBuilder.getPartitionIdColumn(), firstPredicateBuilder.getResourceIdColumn()
+		});
 	}
 
 	public void addOrdering() {
@@ -1582,7 +1586,12 @@ public Condition createPredicateReferenceForEmbeddedChainedSearchResource(
 			if (theSourceJoinColumn == null) {
 				BaseJoiningPredicateBuilder root = mySqlBuilder.getOrCreateFirstPredicateBuilder(false);
 				DbColumn[] joinColumns = root.getJoinColumns();
-				Object joinColumnObject = ColumnTupleObject.from(joinColumns);
+				Object joinColumnObject;
+				if (joinColumns.length == 1) {
+					joinColumnObject = joinColumns[0];
+				} else {
+					joinColumnObject = ColumnTupleObject.from(joinColumns);
+				}
 				retVal = new InCondition(joinColumnObject, union);
 			} else {
 				// -- for the resource link, need join with target_resource_id

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

@@ -128,6 +128,8 @@
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.transaction.support.TransactionSynchronizationManager;
 
+import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -144,7 +146,7 @@
 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 ca.uhn.fhir.jpa.util.InClauseNormalizer.*;
+import static ca.uhn.fhir.jpa.util.InClauseNormalizer.normalizeIdListForInClause;
 import static java.util.Objects.requireNonNull;
 import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
 import static org.apache.commons.lang3.StringUtils.defaultString;
@@ -2118,7 +2120,7 @@ private void attemptComboUniqueSpProcessing(
 			 * The loop allows us to create multiple combo index joins if there
 			 * are multiple AND expressions for the related parameters.
 			 */
-			while (validateParamValuesAreValidForComboParam(theRequest, theParams, comboParamNames)) {
+			while (validateParamValuesAreValidForComboParam(theRequest, theParams, comboParamNames, comboParam)) {
 				applyComboSearchParam(theQueryStack, theParams, theRequest, comboParamNames, comboParam);
 			}
 		}
@@ -2214,7 +2216,10 @@ private void applyComboSearchParam(
 	 * (e.g. <code>?date=gt2024-02-01</code>), etc.
 	 */
 	private boolean validateParamValuesAreValidForComboParam(
-			RequestDetails theRequest, @Nonnull SearchParameterMap theParams, List<String> theComboParamNames) {
+			RequestDetails theRequest,
+			@Nonnull SearchParameterMap theParams,
+			List<String> theComboParamNames,
+			RuntimeSearchParam theComboParam) {
 		boolean paramValuesAreValidForCombo = true;
 		List<List<IQueryParameterType>> paramOrValues = new ArrayList<>(theComboParamNames.size());
 
@@ -2275,6 +2280,19 @@ private boolean validateParamValuesAreValidForComboParam(
 					break;
 				}
 			}
+
+			// Date params are not eligible for using composite unique index
+			// as index could contain date with different precision (e.g. DAY, SECOND)
+			if (nextParamDef.getParamType() == RestSearchParameterTypeEnum.DATE
+					&& theComboParam.getComboSearchParamType() == ComboSearchParamType.UNIQUE) {
+				ourLog.debug(
+						"Search with params {} is not a candidate for combo searching - "
+								+ "Unique combo search parameter '{}' has DATE type",
+						theComboParamNames,
+						nextParamName);
+				paramValuesAreValidForCombo = false;
+				break;
+			}
 		}
 
 		if (CartesianProductUtil.calculateCartesianProductSize(paramOrValues) > 500) {

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

@@ -363,10 +363,11 @@ public void addCustomJoin(
 		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));
-
+		for (int i = 0; i < theSourceColumn.length; i += 1) {
+			onCondition.addCondition(BinaryCondition.equalTo(theSourceColumn[i], theTargetColumn[i]));
+		}
 		return onCondition;
 	}
 
@@ -498,10 +499,6 @@ public void addJoin(
 		mySelect.addJoins(theJoinType, join);
 	}
 
-	public boolean isSelectPartitionId() {
-		return mySelectPartitionId;
-	}
-
 	/**
 	 * Generate and return the SQL generated by this builder
 	 */

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

@@ -108,15 +108,14 @@ private void fetchNext() {
 					 * is managed by Spring has been started before this method is called.
 					 */
 					HapiTransactionService.requireTransaction();
+					ourLog.trace("About to execute SQL: {}. Parameters: {}", sql, Arrays.toString(args));
 
 					Query nativeQuery = myEntityManager.createNativeQuery(sql);
 					org.hibernate.query.Query<?> hibernateQuery = (org.hibernate.query.Query<?>) nativeQuery;
 					for (int i = 1; i <= args.length; i++) {
 						hibernateQuery.setParameter(i, args[i - 1]);
 					}
 
-					ourLog.trace("About to execute SQL: {}. Parameters: {}", sql, Arrays.toString(args));
-
 					/*
 					 * These settings help to ensure that we use a search cursor
 					 * as opposed to loading all search results into memory
@@ -146,24 +145,7 @@ private void fetchNext() {
 				if (myResultSet == null || !myResultSet.hasNext()) {
 					myNext = NO_MORE;
 				} else {
-					Object nextRow = Objects.requireNonNull(myResultSet.next());
-					// We should typically get two columns back, the first is the partition ID and the second
-					// is the resource ID. But if we're doing a count query, we'll get a single column in an array
-					// or maybe even just a single non array value depending on how the platform handles it.
-					if (nextRow instanceof Number) {
-						long count = ((Number) nextRow).longValue();
-						myNext = JpaPid.fromId(count, null);
-					} else {
-						Object[] nextRowAsArray = (Object[]) nextRow;
-						if (nextRowAsArray.length == 1) {
-							Long count = (Long) nextRowAsArray[0];
-							myNext = JpaPid.fromId(count, null);
-						} else {
-							Integer nextPartitionId = (Integer) nextRowAsArray[0];
-							Long nextResourceId = (Long) nextRowAsArray[1];
-							myNext = JpaPid.fromId(nextResourceId, nextPartitionId);
-						}
-					}
+					myNext = getNextPid(myResultSet);
 				}
 
 			} catch (Exception e) {
@@ -174,6 +156,41 @@ private void fetchNext() {
 		}
 	}
 
+	private JpaPid getNextPid(ScrollableResultsIterator<Object> theResultSet) {
+		Object nextRow = Objects.requireNonNull(theResultSet.next());
+		// We should typically get two columns back, the first is the partition ID and the second
+		// is the resource ID. But if we're doing a count query, we'll get a single column in an array
+		// or maybe even just a single non array value depending on how the platform handles it.
+		if (nextRow instanceof Number) {
+			return JpaPid.fromId(((Number) nextRow).longValue());
+		} else {
+			Object[] nextRowAsArray = (Object[]) nextRow;
+			if (nextRowAsArray.length == 1) {
+				return JpaPid.fromId((Long) nextRowAsArray[0]);
+			} else {
+				// TODO MB add a strategy object to GeneratedSql to describe the result set.
+				// or make SQE generic
+				// Comment to reviewer: this will be cleaner with the next
+				// merge from ja_20240718_pk_schema_selector
+
+				// We have some cases to distinguish:
+				// - res_id
+				// - count
+				// - partition_id, res_id
+				// - res_id, coord-dist
+				// - partition_id, res_id, coord-dist
+				// Assume res_id is first Long in row, and is in first two columns
+				if (nextRowAsArray[0] instanceof Long) {
+					return JpaPid.fromId((Long) nextRowAsArray[0]);
+				} else {
+					Integer nextPartitionId = (Integer) nextRowAsArray[0];
+					Long nextResourceId = (Long) nextRowAsArray[1];
+					return JpaPid.fromId(nextResourceId, nextPartitionId);
+				}
+			}
+		}
+	}
+
 	public static SearchQueryExecutor emptyExecutor() {
 		return NO_VALUE_EXECUTOR;
 	}

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java

@@ -37,12 +37,19 @@ public class PartitionSettings {
 
 	public PartitionSettings() {}
 
-	// FIXME: document
+	/**
+	 * Is the table partition column (usually PARTITION_ID)
+	 * participating in primary and foreign keys.
+	 * Affects sql joins, sql in() expressions, etc.
+	 */
 	public boolean isPartitionIdsInPrimaryKeys() {
 		return myPartitionIdsInPrimaryKeys;
 	}
 
-	// FIXME: document
+	/**
+	 * Inform the query engine if the primary and foreign keys
+	 * of the partitioned tables include the partition column.
+	 */
 	public void setPartitionIdsInPrimaryKeys(boolean thePartitionIdsInPrimaryKeys) {
 		myPartitionIdsInPrimaryKeys = thePartitionIdsInPrimaryKeys;
 	}

hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/dao/JpaPidTest.java

@@ -0,0 +1,24 @@
+package ca.uhn.fhir.jpa.model.dao;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static ca.uhn.fhir.jpa.model.dao.JpaPid.fromId;
+import static org.junit.jupiter.api.Assertions.*;
+
+class JpaPidTest {
+
+	@Test
+	void testToLongList() {
+		assertEquals(List.of(), JpaPid.toLongList(List.of()));
+	    assertEquals(List.of(1L, 2L), JpaPid.toLongList(List.of(fromId(1L), fromId(2L))));
+	}
+
+	@Test
+	void testArrayToLongList() {
+		assertEquals(List.of(), JpaPid.toLongList(new JpaPid[0]));
+		assertEquals(List.of(1L, 2L), JpaPid.toLongList(new JpaPid[]{fromId(1L), fromId(2L)}));
+	}
+
+}

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

@@ -566,7 +566,8 @@ private Set<String> extractParameterCombinationsForComboParam(
 				for (BaseResourceIndexedSearchParam nextParam : paramsListForCompositePart) {
 					IQueryParameterType nextParamAsClientParam = nextParam.toQueryParameterType();
 
-					if (nextParamAsClientParam instanceof DateParam) {
+					if (theParam.getComboSearchParamType() == ComboSearchParamType.NON_UNIQUE
+							&& nextParamAsClientParam instanceof DateParam) {
 						DateParam date = (DateParam) nextParamAsClientParam;
 						if (date.getPrecision() != TemporalPrecisionEnum.DAY) {
 							continue;

hapi-fhir-jpaserver-subscription/pom.xml

@@ -70,6 +70,11 @@
 			<artifactId>jakarta.servlet-api</artifactId>
 			<scope>provided</scope>
 		</dependency>
+		<dependency>
+			<groupId>jakarta.mail</groupId>
+			<artifactId>jakarta.mail-api</artifactId>
+			<optional>true</optional>
+		</dependency>
 
 		<!-- test dependencies -->
 		<dependency>

hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu2Test.java

@@ -28,8 +28,8 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeMessage;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderSearchModifierDstu3Test.java

@@ -1,6 +1,5 @@
 package ca.uhn.fhir.jpa.provider.dstu3;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
 import ca.uhn.fhir.jpa.searchparam.ResourceSearch;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
@@ -16,7 +15,7 @@
 
 import java.util.Collections;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class ResourceProviderSearchModifierDstu3Test extends BaseResourceProviderDstu3Test{
 	@Autowired

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSenderImplTest.java

@@ -17,8 +17,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeMessage;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
 import java.util.Arrays;
 
 import static org.assertj.core.api.Assertions.assertThat;

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu3Test.java

@@ -26,8 +26,8 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.beans.factory.annotation.Autowired;
 
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeMessage;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;