Use new FHIR_ID column for sorting

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5405-use-new-fhir-id-for-sort.yaml

@@ -0,0 +1,4 @@
+---
+type: perf
+issue: 5405
+title: "Sorting by _id now uses the FHIR_ID column on HFJ_RESOURCE and avoid joins."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialect.java

@@ -96,7 +96,7 @@ private DataAccessException convertHibernateAccessException(
 							+ makeErrorMessage(
 									messageToPrepend, "resourceIndexedCompositeStringUniqueConstraintFailure"));
 				}
-				if (constraintName.contains(ResourceTable.IDX_RES_FHIR_ID)) {
+				if (constraintName.contains(ResourceTable.IDX_RES_TYPE_FHIR_ID)) {
 					throw new ResourceVersionConflictException(
 							Msg.code(825) + makeErrorMessage(messageToPrepend, "forcedIdConstraintFailure"));
 				}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java

@@ -114,7 +114,6 @@
 import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder;
-import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.QuantityNormalizedPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.QuantityPredicateBuilder;
@@ -613,12 +612,6 @@ public DatePredicateBuilder newDatePredicateBuilder(SearchQueryBuilder theSearch
 		return new DatePredicateBuilder(theSearchBuilder);
 	}
 
-	@Bean
-	@Scope("prototype")
-	public ForcedIdPredicateBuilder newForcedIdPredicateBuilder(SearchQueryBuilder theSearchBuilder) {
-		return new ForcedIdPredicateBuilder(theSearchBuilder);
-	}
-
 	@Bean
 	@Scope("prototype")
 	public NumberPredicateBuilder newNumberPredicateBuilder(SearchQueryBuilder theSearchBuilder) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java

@@ -118,12 +118,19 @@ protected void init700() {
 
 		Builder.BuilderWithTableName hfjResource = version.onTable("HFJ_RESOURCE");
 		hfjResource.modifyColumn("20231018.2", "FHIR_ID").nonNullable();
+
+		hfjResource.dropIndex("20231027.1", "IDX_RES_FHIR_ID");
 		hfjResource
-				.addIndex("20231018.3", "IDX_RES_FHIR_ID")
+				.addIndex("20231027.2", "IDX_RES_TYPE_FHIR_ID")
 				.unique(true)
 				.online(true)
-				.includeColumns("RES_ID")
-				.withColumns("FHIR_ID", "RES_TYPE");
+				// include res_id and our deleted flag so we can satisfy Observation?_sort=_id from the index on
+				// platforms that support it.
+				.includeColumns("RES_ID, RES_DELETED_AT")
+				.withColumns("RES_TYPE", "FHIR_ID");
+
+		// For resolving references that don't supply the type.
+		hfjResource.addIndex("20231027.3", "IDX_RES_FHIR_ID").unique(false).withColumns("FHIR_ID");
 	}
 
 	protected void init680() {

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

@@ -43,7 +43,6 @@
 import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder;
-import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.ICanMakeMissingParamPredicate;
 import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.ParsedLocationParam;
@@ -95,7 +94,6 @@
 import com.healthmarketscience.sqlbuilder.Condition;
 import com.healthmarketscience.sqlbuilder.Expression;
 import com.healthmarketscience.sqlbuilder.InCondition;
-import com.healthmarketscience.sqlbuilder.OrderObject;
 import com.healthmarketscience.sqlbuilder.SelectQuery;
 import com.healthmarketscience.sqlbuilder.SetOperationQuery;
 import com.healthmarketscience.sqlbuilder.Subquery;
@@ -276,16 +274,15 @@ public void addSortOnQuantity(String theResourceName, String theParamName, boole
 	}
 
 	public void addSortOnResourceId(boolean theAscending) {
+		ResourceTablePredicateBuilder resourceTablePredicateBuilder;
 		BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder();
-		ForcedIdPredicateBuilder sortPredicateBuilder =
-				mySqlBuilder.addForcedIdPredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
-		if (!theAscending) {
-			mySqlBuilder.addSortString(
-					sortPredicateBuilder.getColumnForcedId(), false, OrderObject.NullOrder.FIRST, myUseAggregate);
+		if (firstPredicateBuilder instanceof ResourceTablePredicateBuilder) {
+			resourceTablePredicateBuilder = (ResourceTablePredicateBuilder) firstPredicateBuilder;
 		} else {
-			mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), true, myUseAggregate);
+			resourceTablePredicateBuilder =
+					mySqlBuilder.addResourceTablePredicateBuilder(firstPredicateBuilder.getResourceIdColumn());
 		}
-		mySqlBuilder.addSortNumeric(firstPredicateBuilder.getResourceIdColumn(), theAscending, myUseAggregate);
+		mySqlBuilder.addSortString(resourceTablePredicateBuilder.getColumnFhirId(), theAscending, myUseAggregate);
 	}
 
 	public void addSortOnResourceLink(

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

@@ -19,6 +19,7 @@
  */
 package ca.uhn.fhir.jpa.search.builder.predicate;
 
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder;
 import ca.uhn.fhir.jpa.util.QueryParameterUtils;
 import com.healthmarketscience.sqlbuilder.BinaryCondition;
@@ -35,17 +36,19 @@ public class ResourceTablePredicateBuilder extends BaseJoiningPredicateBuilder {
 	private final DbColumn myColumnResType;
 	private final DbColumn myColumnLastUpdated;
 	private final DbColumn myColumnLanguage;
+	private final DbColumn myColumnFhirId;
 
 	/**
 	 * Constructor
 	 */
 	public ResourceTablePredicateBuilder(SearchQueryBuilder theSearchSqlBuilder) {
 		super(theSearchSqlBuilder, theSearchSqlBuilder.addTable("HFJ_RESOURCE"));
 		myColumnResId = getTable().addColumn("RES_ID");
-		myColumnResType = getTable().addColumn("RES_TYPE");
+		myColumnResType = getTable().addColumn(ResourceTable.RES_TYPE);
 		myColumnResDeletedAt = getTable().addColumn("RES_DELETED_AT");
 		myColumnLastUpdated = getTable().addColumn("RES_UPDATED");
 		myColumnLanguage = getTable().addColumn("RES_LANGUAGE");
+		myColumnFhirId = getTable().addColumn(ResourceTable.FHIR_ID);
 	}
 
 	@Override
@@ -77,4 +80,8 @@ public Condition createLanguagePredicate(Set<String> theValues, boolean theNegat
 	public DbColumn getColumnLastUpdated() {
 		return myColumnLastUpdated;
 	}
+
+	public DbColumn getColumnFhirId() {
+		return myColumnFhirId;
+	}
 }

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

@@ -32,7 +32,6 @@
 import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder;
-import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.QuantityNormalizedPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.QuantityPredicateBuilder;
@@ -62,7 +61,6 @@
 import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSchema;
 import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSpec;
 import com.healthmarketscience.sqlbuilder.dbspec.basic.DbTable;
-import org.apache.commons.lang3.Validate;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.SQLServerDialect;
 import org.hibernate.dialect.pagination.AbstractLimitHandler;
@@ -222,18 +220,6 @@ public DatePredicateBuilder createDatePredicateBuilder() {
 		return mySqlBuilderFactory.dateIndexTable(this);
 	}
 
-	/**
-	 * Add and return a predicate builder for selecting a forced ID. This is only intended for use with sorts so it can not
-	 * be the root query.
-	 */
-	public ForcedIdPredicateBuilder addForcedIdPredicateBuilder(@Nonnull DbColumn theSourceJoinColumn) {
-		Validate.isTrue(theSourceJoinColumn != null);
-
-		ForcedIdPredicateBuilder retVal = mySqlBuilderFactory.newForcedIdPredicateBuilder(this);
-		addTableForSorting(retVal, theSourceJoinColumn);
-		return retVal;
-	}
-
 	/**
 	 * Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a NUMBER search parameter
 	 */
@@ -417,11 +403,6 @@ private void addTable(BaseJoiningPredicateBuilder thePredicateBuilder, @Nullable
 		addTable(thePredicateBuilder, theSourceJoinColumn, SelectQuery.JoinType.INNER);
 	}
 
-	private void addTableForSorting(
-			BaseJoiningPredicateBuilder thePredicateBuilder, @Nullable DbColumn theSourceJoinColumn) {
-		addTable(thePredicateBuilder, theSourceJoinColumn, SelectQuery.JoinType.LEFT_OUTER);
-	}
-
 	private void addTable(
 			BaseJoiningPredicateBuilder thePredicateBuilder,
 			@Nullable DbColumn theSourceJoinColumn,
@@ -766,7 +747,7 @@ public void excludeResourceIdsPredicate(Set<JpaPid> theExistingPidSetToExclude)
 
 		List<Long> excludePids = JpaPid.toLongList(theExistingPidSetToExclude);
 
-		ourLog.trace("excludePids = " + excludePids);
+		ourLog.trace("excludePids = {}", excludePids);
 
 		DbColumn resourceIdColumn = getOrCreateFirstPredicateBuilder().getResourceIdColumn();
 		InCondition predicate = new InCondition(resourceIdColumn, generatePlaceholders(excludePids));

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

@@ -24,7 +24,6 @@
 import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder;
-import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.QuantityNormalizedPredicateBuilder;
 import ca.uhn.fhir.jpa.search.builder.predicate.QuantityPredicateBuilder;
@@ -63,10 +62,6 @@ public DatePredicateBuilder dateIndexTable(SearchQueryBuilder theSearchSqlBuilde
 		return myApplicationContext.getBean(DatePredicateBuilder.class, theSearchSqlBuilder);
 	}
 
-	public ForcedIdPredicateBuilder newForcedIdPredicateBuilder(SearchQueryBuilder theSearchSqlBuilder) {
-		return myApplicationContext.getBean(ForcedIdPredicateBuilder.class, theSearchSqlBuilder);
-	}
-
 	public NumberPredicateBuilder numberIndexTable(SearchQueryBuilder theSearchSqlBuilder) {
 		return myApplicationContext.getBean(NumberPredicateBuilder.class, theSearchSqlBuilder);
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java

@@ -2979,7 +2979,7 @@ public Optional<IBaseResource> readCodeSystemByForcedId(String theForcedId) {
 
 		if (resultList.size() > 1)
 			throw new NonUniqueResultException(Msg.code(911) + "More than one CodeSystem is pointed by forcedId: "
-					+ theForcedId + ". Was constraint " + ResourceTable.IDX_RES_FHIR_ID + " removed?");
+					+ theForcedId + ". Was constraint " + ResourceTable.IDX_RES_TYPE_FHIR_ID + " removed?");
 
 		IFhirResourceDao<CodeSystem> csDao = myDaoRegistry.getResourceDao("CodeSystem");
 		IBaseResource cs = myJpaStorageResourceParser.toResource(resultList.get(0), false);

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java

@@ -76,20 +76,21 @@
 import javax.persistence.UniqueConstraint;
 import javax.persistence.Version;
 
-import static ca.uhn.fhir.jpa.model.entity.ResourceTable.IDX_RES_FHIR_ID;
+import static ca.uhn.fhir.jpa.model.entity.ResourceTable.IDX_RES_TYPE_FHIR_ID;
 
 @Indexed(routingBinder = @RoutingBinderRef(type = ResourceTableRoutingBinder.class))
 @Entity
 @Table(
 		name = ResourceTable.HFJ_RESOURCE,
 		uniqueConstraints = {
 			@UniqueConstraint(
-					name = IDX_RES_FHIR_ID,
-					columnNames = {"FHIR_ID", "RES_TYPE"})
+					name = IDX_RES_TYPE_FHIR_ID,
+					columnNames = {"RES_TYPE", "FHIR_ID"})
 		},
 		indexes = {
 			// Do not reuse previously used index name: IDX_INDEXSTATUS, IDX_RES_TYPE
 			@Index(name = "IDX_RES_DATE", columnList = BaseHasResource.RES_UPDATED),
+			@Index(name = "IDX_RES_FHIR_ID", columnList = "FHIR_ID"),
 			@Index(
 					name = "IDX_RES_TYPE_DEL_UPDATED",
 					columnList = "RES_TYPE,RES_DELETED_AT,RES_UPDATED,PARTITION_ID,RES_ID"),
@@ -100,10 +101,11 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas
 	public static final int RESTYPE_LEN = 40;
 	public static final String HFJ_RESOURCE = "HFJ_RESOURCE";
 	public static final String RES_TYPE = "RES_TYPE";
+	public static final String FHIR_ID = "FHIR_ID";
 	private static final int MAX_LANGUAGE_LENGTH = 20;
 	private static final long serialVersionUID = 1L;
 	public static final int MAX_FORCED_ID_LENGTH = 100;
-	public static final String IDX_RES_FHIR_ID = "IDX_RES_FHIR_ID";
+	public static final String IDX_RES_TYPE_FHIR_ID = "IDX_RES_TYPE_FHIR_ID";
 
 	/**
 	 * Holds the narrative text only - Used for Fulltext searching but not directly stored in the DB
@@ -381,7 +383,7 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas
 	 * Will be null during insert time until the first read.
 	 */
 	@Column(
-			name = "FHIR_ID",
+			name = FHIR_ID,
 			// [A-Za-z0-9\-\.]{1,64} - https://www.hl7.org/fhir/datatypes.html#id
 			length = 64,
 			// we never update this after insert, and the Generator will otherwise "dirty" the object.

hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java

@@ -2189,21 +2189,21 @@ public void testSortById() {
 		pm.setSort(new SortSpec(BaseResource.SP_RES_ID));
 		actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm));
 		assertEquals(5, actual.size());
-		assertThat(actual, contains(idMethodName, id1, id2, id3, id4));
+		assertThat(actual, contains(id1, id2, id3, id4, idMethodName));
 
 		pm = new SearchParameterMap();
 		pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName));
 		pm.setSort(new SortSpec(BaseResource.SP_RES_ID).setOrder(SortOrderEnum.ASC));
 		actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm));
 		assertEquals(5, actual.size());
-		assertThat(actual, contains(idMethodName, id1, id2, id3, id4));
+		assertThat(actual, contains(id1, id2, id3, id4, idMethodName));
 
 		pm = new SearchParameterMap();
 		pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName));
 		pm.setSort(new SortSpec(BaseResource.SP_RES_ID).setOrder(SortOrderEnum.DESC));
 		actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm));
 		assertEquals(5, actual.size());
-		assertThat(actual, contains(id4, id3, id2, id1, idMethodName));
+		assertThat(actual, contains(idMethodName, id4, id3, id2, id1));
 	}
 
 	@Test

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java

@@ -3323,7 +3323,7 @@ public void testSortOnId() throws Exception {
 		map = new SearchParameterMap();
 		map.setSort(new SortSpec("_id", SortOrderEnum.ASC));
 		ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map));
-		assertThat(ids, contains("Patient/AA", "Patient/AB", id1, id2));
+		assertThat(ids, contains(id1, id2, "Patient/AA", "Patient/AB"));
 
 	}
 

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java

@@ -2788,21 +2788,21 @@ public void testSortById() {
 		pm.setSort(new SortSpec(IAnyResource.SP_RES_ID));
 		actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm));
 		assertEquals(5, actual.size());
-		assertThat(actual.toString(), actual, contains(idMethodName, id1, id2, id3, id4));
+		assertThat(actual.toString(), actual, contains(id1, id2, id3, id4, idMethodName));
 
 		pm = new SearchParameterMap();
 		pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName));
 		pm.setSort(new SortSpec(IAnyResource.SP_RES_ID).setOrder(SortOrderEnum.ASC));
 		actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm));
 		assertEquals(5, actual.size());
-		assertThat(actual.toString(), actual, contains(idMethodName, id1, id2, id3, id4));
+		assertThat(actual.toString(), actual, contains(id1, id2, id3, id4, idMethodName));
 
 		pm = new SearchParameterMap();
 		pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName));
 		pm.setSort(new SortSpec(IAnyResource.SP_RES_ID).setOrder(SortOrderEnum.DESC));
 		actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm));
 		assertEquals(5, actual.size());
-		assertThat(actual.toString(), actual, contains(id4, id3, id2, id1, idMethodName));
+		assertThat(actual.toString(), actual, contains(idMethodName, id4, id3, id2, id1));
 	}
 
 	@Test

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

@@ -138,7 +138,7 @@ protected void createUniqueCompositeSp() {
 	protected void dropForcedIdUniqueConstraint() {
 		runInTransaction(() -> {
 			myEntityManager.createNativeQuery("alter table " + ForcedId.HFJ_FORCED_ID + " drop constraint " + ForcedId.IDX_FORCEDID_TYPE_FID).executeUpdate();
-			myEntityManager.createNativeQuery("alter table " + ResourceTable.HFJ_RESOURCE + " drop constraint " + ResourceTable.IDX_RES_FHIR_ID).executeUpdate();
+			myEntityManager.createNativeQuery("alter table " + ResourceTable.HFJ_RESOURCE + " drop constraint " + ResourceTable.IDX_RES_TYPE_FHIR_ID).executeUpdate();
 		});
 		myHaveDroppedForcedIdUniqueConstraint = true;
 	}