Try to reuse index rows in JPA server (#1133)

* Try to reuse index rows in JPA server

* Address review comments

* One more test fix

* One more test

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -1414,7 +1414,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 		if (thePerformIndexing) {
 			myDatabaseSearchParamSynchronizer.synchronizeSearchParamsToDatabase(newParams, theEntity, existingParams);
 			mySearchParamWithInlineReferencesExtractor.storeCompositeStringUniques(newParams, theEntity, existingParams);
-		} // if thePerformIndexing
+		}
 
 		if (theResource != null) {
 			populateResourceIdFromEntity(theEntity, theResource);

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java

@@ -21,8 +21,8 @@ package ca.uhn.fhir.jpa.dao;
  */
 
 import ca.uhn.fhir.jpa.dao.data.IForcedIdDao;
-import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.dao.index.IdHelperService;
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.model.api.IQueryParameterType;
 import ca.uhn.fhir.rest.api.Constants;
@@ -42,7 +42,7 @@ import org.hibernate.search.jpa.FullTextEntityManager;
 import org.hibernate.search.jpa.FullTextQuery;
 import org.hibernate.search.query.dsl.BooleanJunction;
 import org.hibernate.search.query.dsl.QueryBuilder;
-import org.hl7.fhir.dstu3.model.BaseResource;
+import org.hl7.fhir.instance.model.api.IAnyResource;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.annotation.Transactional;
@@ -181,7 +181,7 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 		addTextSearch(qb, bool, textAndTerms, "myNarrativeText", "myNarrativeTextEdgeNGram", "myNarrativeTextNGram");
 
 		if (theReferencingPid != null) {
-			bool.must(qb.keyword().onField("myResourceLinks.myTargetResourcePid").matching(theReferencingPid).createQuery());
+			bool.must(qb.keyword().onField("myResourceLinksField").matching(theReferencingPid.toString()).createQuery());
 		}
 
 		if (bool.isEmpty()) {
@@ -201,13 +201,11 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 		// execute search
 		List<?> result = jpaQuery.getResultList();
 
-		HashSet<Long> pidsSet = pids != null ? new HashSet<Long>(pids) : null;
+		ArrayList<Long> retVal = new ArrayList<>();
-
-		ArrayList<Long> retVal = new ArrayList<Long>();
 		for (Object object : result) {
 			Object[] nextArray = (Object[]) object;
 			Long next = (Long) nextArray[0];
-			if (next != null && (pidsSet == null || pidsSet.contains(next))) {
+			if (next != null) {
 				retVal.add(next);
 			}
 		}
@@ -219,9 +217,9 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 	public List<Long> everything(String theResourceName, SearchParameterMap theParams) {
 
 		Long pid = null;
-		if (theParams.get(BaseResource.SP_RES_ID) != null) {
+		if (theParams.get(IAnyResource.SP_RES_ID) != null) {
 			String idParamValue;
-			IQueryParameterType idParam = theParams.get(BaseResource.SP_RES_ID).get(0).get(0);
+			IQueryParameterType idParam = theParams.get(IAnyResource.SP_RES_ID).get(0).get(0);
 			if (idParam instanceof TokenParam) {
 				TokenParam idParm = (TokenParam) idParam;
 				idParamValue = idParm.getValue();
@@ -298,7 +296,8 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 			.sentence(theText.toLowerCase()).createQuery();
 
 		Query query = qb.bool()
-			.must(qb.keyword().onField("myResourceLinks.myTargetResourcePid").matching(pid).createQuery())
+//			.must(qb.keyword().onField("myResourceLinks.myTargetResourcePid").matching(pid).createQuery())
+			.must(qb.keyword().onField("myResourceLinksField").matching(pid.toString()).createQuery())
 			.must(textQuery)
 			.createQuery();
 
@@ -345,7 +344,7 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 		}
 
 		long delay = System.currentTimeMillis() - start;
-		ourLog.info("Provided {} suggestions for term {} in {} ms", new Object[]{terms.size(), theText, delay});
+		ourLog.info("Provided {} suggestions for term {} in {} ms", terms.size(), theText, delay);
 
 		return suggestions;
 	}
@@ -358,14 +357,14 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 		private ArrayList<Float> myPartialMatchScores;
 		private String myOriginalSearch;
 
-		public MySuggestionFormatter(String theOriginalSearch, List<Suggestion> theSuggestions) {
+		MySuggestionFormatter(String theOriginalSearch, List<Suggestion> theSuggestions) {
 			myOriginalSearch = theOriginalSearch;
 			mySuggestions = theSuggestions;
 		}
 
 		@Override
 		public String highlightTerm(String theOriginalText, TokenGroup theTokenGroup) {
-			ourLog.debug("{} Found {} with score {}", new Object[]{myAnalyzer, theOriginalText, theTokenGroup.getTotalScore()});
+			ourLog.debug("{} Found {} with score {}", myAnalyzer, theOriginalText, theTokenGroup.getTotalScore());
 			if (theTokenGroup.getTotalScore() > 0) {
 				float score = theTokenGroup.getTotalScore();
 				if (theOriginalText.equalsIgnoreCase(myOriginalSearch)) {
@@ -385,13 +384,13 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 			return null;
 		}
 
-		public void setAnalyzer(String theString) {
+		void setAnalyzer(String theString) {
 			myAnalyzer = theString;
 		}
 
-		public void setFindPhrasesWith() {
+		void setFindPhrasesWith() {
-			myPartialMatchPhrases = new ArrayList<String>();
+			myPartialMatchPhrases = new ArrayList<>();
-			myPartialMatchScores = new ArrayList<Float>();
+			myPartialMatchScores = new ArrayList<>();
 
 			for (Suggestion next : mySuggestions) {
 				myPartialMatchPhrases.add(' ' + next.myTerm);
@@ -408,7 +407,7 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
 		private String myTerm;
 		private float myScore;
 
-		public Suggestion(String theTerm, float theScore) {
+		Suggestion(String theTerm, float theScore) {
 			myTerm = theTerm;
 			myScore = theScore;
 		}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DatabaseSearchParamSynchronizer.java

@@ -23,6 +23,7 @@ package ca.uhn.fhir.jpa.dao.index;
 import ca.uhn.fhir.jpa.dao.DaoConfig;
 import ca.uhn.fhir.jpa.model.entity.*;
 import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams;
+import org.apache.commons.lang3.Validate;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
@@ -31,6 +32,7 @@ import javax.persistence.PersistenceContext;
 import javax.persistence.PersistenceContextType;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 @Service
 public class DatabaseSearchParamSynchronizer {
@@ -41,95 +43,75 @@ public class DatabaseSearchParamSynchronizer {
 	protected EntityManager myEntityManager;
 
 	public void synchronizeSearchParamsToDatabase(ResourceIndexedSearchParams theParams, ResourceTable theEntity, ResourceIndexedSearchParams existingParams) {
-		theParams.calculateHashes(theParams.stringParams);
-		for (ResourceIndexedSearchParamString next : synchronizeSearchParamsToDatabase(existingParams.stringParams, theParams.stringParams)) {
-			next.setModelConfig(myDaoConfig.getModelConfig());
-			myEntityManager.remove(next);
-			theEntity.getParamsString().remove(next);
-		}
-		for (ResourceIndexedSearchParamString next : synchronizeSearchParamsToDatabase(theParams.stringParams, existingParams.stringParams)) {
-			myEntityManager.persist(next);
-		}
 
-		theParams.calculateHashes(theParams.tokenParams);
+		synchronize(theParams, theEntity, theParams.stringParams, existingParams.stringParams);
-		for (ResourceIndexedSearchParamToken next : synchronizeSearchParamsToDatabase(existingParams.tokenParams, theParams.tokenParams)) {
+		synchronize(theParams, theEntity, theParams.tokenParams, existingParams.tokenParams);
-			myEntityManager.remove(next);
+		synchronize(theParams, theEntity, theParams.numberParams, existingParams.numberParams);
-			theEntity.getParamsToken().remove(next);
+		synchronize(theParams, theEntity, theParams.quantityParams, existingParams.quantityParams);
-		}
+		synchronize(theParams, theEntity, theParams.dateParams, existingParams.dateParams);
-		for (ResourceIndexedSearchParamToken next : synchronizeSearchParamsToDatabase(theParams.tokenParams, existingParams.tokenParams)) {
+		synchronize(theParams, theEntity, theParams.uriParams, existingParams.uriParams);
-			myEntityManager.persist(next);
+		synchronize(theParams, theEntity, theParams.coordsParams, existingParams.coordsParams);
-		}
+		synchronize(theParams, theEntity, theParams.links, existingParams.links);
-
-		theParams.calculateHashes(theParams.numberParams);
-		for (ResourceIndexedSearchParamNumber next : synchronizeSearchParamsToDatabase(existingParams.numberParams, theParams.numberParams)) {
-			myEntityManager.remove(next);
-			theEntity.getParamsNumber().remove(next);
-		}
-		for (ResourceIndexedSearchParamNumber next : synchronizeSearchParamsToDatabase(theParams.numberParams, existingParams.numberParams)) {
-			myEntityManager.persist(next);
-		}
-
-		theParams.calculateHashes(theParams.quantityParams);
-		for (ResourceIndexedSearchParamQuantity next : synchronizeSearchParamsToDatabase(existingParams.quantityParams, theParams.quantityParams)) {
-			myEntityManager.remove(next);
-			theEntity.getParamsQuantity().remove(next);
-		}
-		for (ResourceIndexedSearchParamQuantity next : synchronizeSearchParamsToDatabase(theParams.quantityParams, existingParams.quantityParams)) {
-			myEntityManager.persist(next);
-		}
-
-		// Store date SP's
-		theParams.calculateHashes(theParams.dateParams);
-		for (ResourceIndexedSearchParamDate next : synchronizeSearchParamsToDatabase(existingParams.dateParams, theParams.dateParams)) {
-			myEntityManager.remove(next);
-			theEntity.getParamsDate().remove(next);
-		}
-		for (ResourceIndexedSearchParamDate next : synchronizeSearchParamsToDatabase(theParams.dateParams, existingParams.dateParams)) {
-			myEntityManager.persist(next);
-		}
-
-		// Store URI SP's
-		theParams.calculateHashes(theParams.uriParams);
-		for (ResourceIndexedSearchParamUri next : synchronizeSearchParamsToDatabase(existingParams.uriParams, theParams.uriParams)) {
-			myEntityManager.remove(next);
-			theEntity.getParamsUri().remove(next);
-		}
-		for (ResourceIndexedSearchParamUri next : synchronizeSearchParamsToDatabase(theParams.uriParams, existingParams.uriParams)) {
-			myEntityManager.persist(next);
-		}
-
-		// Store Coords SP's
-		theParams.calculateHashes(theParams.coordsParams);
-		for (ResourceIndexedSearchParamCoords next : synchronizeSearchParamsToDatabase(existingParams.coordsParams, theParams.coordsParams)) {
-			myEntityManager.remove(next);
-			theEntity.getParamsCoords().remove(next);
-		}
-		for (ResourceIndexedSearchParamCoords next : synchronizeSearchParamsToDatabase(theParams.coordsParams, existingParams.coordsParams)) {
-			myEntityManager.persist(next);
-		}
-
-		// Store resource links
-		for (ResourceLink next : synchronizeSearchParamsToDatabase(existingParams.links, theParams.links)) {
-			myEntityManager.remove(next);
-			theEntity.getResourceLinks().remove(next);
-		}
-		for (ResourceLink next : synchronizeSearchParamsToDatabase(theParams.links, existingParams.links)) {
-			myEntityManager.persist(next);
-		}
 
 		// make sure links are indexed
 		theEntity.setResourceLinks(theParams.links);
 	}
 
-	public <T> Collection<T> synchronizeSearchParamsToDatabase(Collection<T> theInput, Collection<T> theToRemove) {
+	private <T extends BaseResourceIndex> void synchronize(ResourceIndexedSearchParams theParams, ResourceTable theEntity, Collection<T> theNewParms, Collection<T> theExistingParms) {
-		assert theInput != theToRemove;
+		theParams.calculateHashes(theNewParms);
+		List<T> quantitiesToRemove = subtract(theExistingParms, theNewParms);
+		List<T> quantitiesToAdd = subtract(theNewParms, theExistingParms);
+		tryToReuseIndexEntities(quantitiesToRemove, quantitiesToAdd);
+		for (T next : quantitiesToRemove) {
+			myEntityManager.remove(next);
+			theEntity.getParamsQuantity().remove(next);
+		}
+		for (T next : quantitiesToAdd) {
+			myEntityManager.merge(next);
+		}
+	}
 
-		if (theInput.isEmpty()) {
+	/**
-			return theInput;
+	 * The logic here is that often times when we update a resource we are dropping
+	 * one index row and adding another. This method tries to reuse rows that would otherwise
+	 * have been deleted by updating them with the contents of rows that would have
+	 * otherwise been added. In other words, we're trying to replace
+	 * "one delete + one insert" with "one update"
+	 *
+	 * @param theIndexesToRemove The rows that would be removed
+	 * @param theIndexesToAdd The rows that would be added
+	 */
+	private <T extends BaseResourceIndex> void tryToReuseIndexEntities(List<T> theIndexesToRemove, List<T> theIndexesToAdd) {
+		for (int addIndex = 0; addIndex < theIndexesToAdd.size(); addIndex++) {
+
+			// If there are no more rows to remove, there's nothing we can reuse
+			if (theIndexesToRemove.isEmpty()) {
+				break;
+			}
+
+			T targetEntity = theIndexesToAdd.get(addIndex);
+			if (targetEntity.getId() != null) {
+				continue;
+			}
+
+			// Take a row we were going to remove, and repurpose its ID
+			T entityToReuse = theIndexesToRemove.remove(theIndexesToRemove.size() - 1);
+			targetEntity.setId(entityToReuse.getId());
+		}
+	}
+
+
+
+
+	<T> List<T> subtract(Collection<T> theSubtractFrom, Collection<T> theToSubtract) {
+		assert theSubtractFrom != theToSubtract;
+
+		if (theSubtractFrom.isEmpty()) {
+			return new ArrayList<>();
 		}
 
-		ArrayList<T> retVal = new ArrayList<>(theInput);
+		ArrayList<T> retVal = new ArrayList<>(theSubtractFrom);
-		retVal.removeAll(theToRemove);
+		retVal.removeAll(theToSubtract);
 		return retVal;
 	}
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java

@@ -258,12 +258,12 @@ public class SearchParamWithInlineReferencesExtractor {
 
 		// Store composite string uniques
 		if (myDaoConfig.isUniqueIndexesEnabled()) {
-			for (ResourceIndexedCompositeStringUnique next : myDatabaseSearchParamSynchronizer.synchronizeSearchParamsToDatabase(existingParams.compositeStringUniques, theParams.compositeStringUniques)) {
+			for (ResourceIndexedCompositeStringUnique next : myDatabaseSearchParamSynchronizer.subtract(existingParams.compositeStringUniques, theParams.compositeStringUniques)) {
 				ourLog.debug("Removing unique index: {}", next);
 				myEntityManager.remove(next);
 				theEntity.getParamsCompositeStringUnique().remove(next);
 			}
-			for (ResourceIndexedCompositeStringUnique next : myDatabaseSearchParamSynchronizer.synchronizeSearchParamsToDatabase(theParams.compositeStringUniques, existingParams.compositeStringUniques)) {
+			for (ResourceIndexedCompositeStringUnique next : myDatabaseSearchParamSynchronizer.subtract(theParams.compositeStringUniques, existingParams.compositeStringUniques)) {
 				if (myDaoConfig.isUniqueIndexesCheckedBeforeSave()) {
 					ResourceIndexedCompositeStringUnique existing = myResourceIndexedCompositeStringUniqueDao.findByQueryString(next.getIndexString());
 					if (existing != null) {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java

@@ -102,7 +102,7 @@ public class TestR4Config extends BaseJavaConfigR4 {
 
 		DataSource dataSource = ProxyDataSourceBuilder
 			.create(retVal)
-//			.logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL")
+			.logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL")
 			.logSlowQueryBySlf4j(10, TimeUnit.SECONDS)
 //			.countQuery(new ThreadQueryCountHolder())
 			.countQuery(singleQueryCountHolder())

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

@@ -3,11 +3,16 @@ package ca.uhn.fhir.jpa.dao.r4;
 import ca.uhn.fhir.jpa.dao.DaoConfig;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.param.ReferenceParam;
+import ca.uhn.fhir.rest.param.StringParam;
+import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.util.TestUtil;
 import net.ttddyy.dsproxy.QueryCount;
 import net.ttddyy.dsproxy.listener.SingleQueryCountHolder;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.DateTimeType;
+import org.hl7.fhir.r4.model.Enumerations;
+import org.hl7.fhir.r4.model.Organization;
 import org.hl7.fhir.r4.model.Patient;
 import org.junit.After;
 import org.junit.AfterClass;
@@ -63,9 +68,9 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 
 		ourLog.info("** Done performing write 2");
 
-		assertEquals(2, getQueryCount().getInsert());
+		assertEquals(1, getQueryCount().getInsert());
-		assertEquals(1, getQueryCount().getUpdate());
+		assertEquals(2, getQueryCount().getUpdate());
-		assertEquals(1, getQueryCount().getDelete());
+		assertEquals(0, getQueryCount().getDelete());
 	}
 
 	@Test
@@ -128,7 +133,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2012")); // non-indexed field
 		myPatientDao.update(p).getId().toUnqualifiedVersionless();
 
-		assertEquals(5, getQueryCount().getSelect());
+		assertEquals(4, getQueryCount().getSelect());
 		assertEquals(1, getQueryCount().getInsert());
 		assertEquals(0, getQueryCount().getDelete());
 		assertEquals(1, getQueryCount().getUpdate());
@@ -157,6 +162,8 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 			assertEquals(1, myResourceHistoryTableDao.count());
 		});
 
+
+
 		myCountHolder.clear();
 		p = new Patient();
 		p.setId(id);
@@ -183,8 +190,6 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 		pt.addName().setFamily("FAMILY1").addGiven("GIVEN1A").addGiven("GIVEN1B");
 		IIdType id = myPatientDao.create(pt).getId().toUnqualifiedVersionless();
 
-		ourLog.info("Now have {} deleted", getQueryCount().getDelete());
-		ourLog.info("Now have {} inserts", getQueryCount().getInsert());
 		myCountHolder.clear();
 
 		ourLog.info("** About to update");
@@ -193,13 +198,148 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
 		pt.getNameFirstRep().addGiven("GIVEN1C");
 		myPatientDao.update(pt);
 
-		ourLog.info("Now have {} deleted", getQueryCount().getDelete());
-		ourLog.info("Now have {} inserts", getQueryCount().getInsert());
 		assertEquals(0, getQueryCount().getDelete());
 		assertEquals(2, getQueryCount().getInsert());
 	}
 
 
+	@Test
+	public void testUpdateReusesIndexesString() {
+		myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
+		SearchParameterMap m1 = new SearchParameterMap().add("family", new StringParam("family1")).setLoadSynchronous(true);
+		SearchParameterMap m2 = new SearchParameterMap().add("family", new StringParam("family2")).setLoadSynchronous(true);
+
+		myCountHolder.clear();
+
+		Patient pt = new Patient();
+		pt.addName().setFamily("FAMILY1");
+		IIdType id = myPatientDao.create(pt).getId().toUnqualifiedVersionless();
+
+		myCountHolder.clear();
+
+		assertEquals(1, myPatientDao.search(m1).size().intValue());
+		assertEquals(0, myPatientDao.search(m2).size().intValue());
+
+		ourLog.info("** About to update");
+
+		pt = new Patient();
+		pt.setId(id);
+		pt.addName().setFamily("FAMILY2");
+		myPatientDao.update(pt);
+
+		assertEquals(0, getQueryCount().getDelete());
+		assertEquals(1, getQueryCount().getInsert()); // Add an entry to HFJ_RES_VER
+		assertEquals(2, getQueryCount().getUpdate()); // Update SPIDX_STRING and HFJ_RESOURCE
+
+		assertEquals(0, myPatientDao.search(m1).size().intValue());
+		assertEquals(1, myPatientDao.search(m2).size().intValue());
+	}
+
+
+	@Test
+	public void testUpdateReusesIndexesToken() {
+		myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
+		SearchParameterMap m1 = new SearchParameterMap().add("gender", new TokenParam("male")).setLoadSynchronous(true);
+		SearchParameterMap m2 = new SearchParameterMap().add("gender", new TokenParam("female")).setLoadSynchronous(true);
+
+		myCountHolder.clear();
+
+		Patient pt = new Patient();
+		pt.setGender(Enumerations.AdministrativeGender.MALE);
+		IIdType id = myPatientDao.create(pt).getId().toUnqualifiedVersionless();
+
+		assertEquals(0, getQueryCount().getSelect());
+		assertEquals(0, getQueryCount().getDelete());
+		assertEquals(3, getQueryCount().getInsert());
+		assertEquals(0, getQueryCount().getUpdate());
+		assertEquals(1, myPatientDao.search(m1).size().intValue());
+		assertEquals(0, myPatientDao.search(m2).size().intValue());
+
+		/*
+		 * Change a value
+		 */
+
+		ourLog.info("** About to update");
+		myCountHolder.clear();
+
+		pt = new Patient();
+		pt.setId(id);
+		pt.setGender(Enumerations.AdministrativeGender.FEMALE);
+		myPatientDao.update(pt);
+
+		/*
+		 * Current SELECTs:
+		 *   Select the resource from HFJ_RESOURCE
+		 *   Select the version from HFJ_RES_VER
+		 *   Select the current token indexes
+		 */
+		assertEquals(3, getQueryCount().getSelect());
+		assertEquals(0, getQueryCount().getDelete());
+		assertEquals(1, getQueryCount().getInsert()); // Add an entry to HFJ_RES_VER
+		assertEquals(2, getQueryCount().getUpdate()); // Update SPIDX_STRING and HFJ_RESOURCE
+
+		assertEquals(0, myPatientDao.search(m1).size().intValue());
+		assertEquals(1, myPatientDao.search(m2).size().intValue());
+		myCountHolder.clear();
+
+		/*
+		 * Drop a value
+		 */
+
+		ourLog.info("** About to update again");
+
+		pt = new Patient();
+		pt.setId(id);
+		myPatientDao.update(pt);
+
+		assertEquals(1, getQueryCount().getDelete());
+		assertEquals(1, getQueryCount().getInsert());
+		assertEquals(1, getQueryCount().getUpdate());
+
+		assertEquals(0, myPatientDao.search(m1).size().intValue());
+		assertEquals(0, myPatientDao.search(m2).size().intValue());
+
+	}
+
+	@Test
+	public void testUpdateReusesIndexesResourceLink() {
+		Organization org1 = new Organization();
+		org1.setName("org1");
+		IIdType orgId1 = myOrganizationDao.create(org1).getId().toUnqualifiedVersionless();
+		Organization org2 = new Organization();
+		org2.setName("org2");
+		IIdType orgId2 = myOrganizationDao.create(org2).getId().toUnqualifiedVersionless();
+
+		myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
+		SearchParameterMap m1 = new SearchParameterMap().add("organization", new ReferenceParam(orgId1.getValue())).setLoadSynchronous(true);
+		SearchParameterMap m2 = new SearchParameterMap().add("organization", new ReferenceParam(orgId2.getValue())).setLoadSynchronous(true);
+
+		myCountHolder.clear();
+
+		Patient pt = new Patient();
+		pt.getManagingOrganization().setReference(orgId1.getValue());
+		IIdType id = myPatientDao.create(pt).getId().toUnqualifiedVersionless();
+
+		myCountHolder.clear();
+
+		assertEquals(1, myPatientDao.search(m1).size().intValue());
+		assertEquals(0, myPatientDao.search(m2).size().intValue());
+
+		ourLog.info("** About to update");
+
+		pt = new Patient();
+		pt.setId(id);
+		pt.getManagingOrganization().setReference(orgId2.getValue());
+		myPatientDao.update(pt);
+
+		assertEquals(0, getQueryCount().getDelete());
+		assertEquals(1, getQueryCount().getInsert()); // Add an entry to HFJ_RES_VER
+		assertEquals(2, getQueryCount().getUpdate()); // Update SPIDX_STRING and HFJ_RESOURCE
+
+		assertEquals(0, myPatientDao.search(m1).size().intValue());
+		assertEquals(1, myPatientDao.search(m2).size().intValue());
+	}
+
 	@AfterClass
 	public static void afterClassClearContext() {
 		TestUtil.clearAllStaticFieldsForUnitTest();

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/SystemProviderR4Test.java

@@ -4,11 +4,14 @@ import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test;
 import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test;
 import ca.uhn.fhir.jpa.rp.r4.*;
+import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider;
 import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.rest.api.EncodingEnum;
+import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.client.api.IGenericClient;
 import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor;
+import ca.uhn.fhir.rest.param.ReferenceParam;
 import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
 import ca.uhn.fhir.rest.server.RestfulServer;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
@@ -411,6 +414,13 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
 
 			myPatientDao.read(new IdType("Patient/Patient1063259"));
 
+
+			SearchParameterMap params = new SearchParameterMap();
+			params.add("subject", new ReferenceParam("Patient1063259"));
+			params.setLoadSynchronous(true);
+			IBundleProvider result = myDiagnosticReportDao.search(params);
+			assertEquals(1, result.size().intValue());
+
 			deleteAllOfType("Binary");
 			deleteAllOfType("Location");
 			deleteAllOfType("DiagnosticReport");
@@ -427,6 +437,9 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
 				// good
 			}
 
+			result = myDiagnosticReportDao.search(params);
+			assertEquals(0, result.size().intValue());
+
 		}
 
 	}

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

@@ -0,0 +1,25 @@
+package ca.uhn.fhir.jpa.model.entity;
+
+import java.io.Serializable;
+
+public abstract class BaseResourceIndex implements Serializable {
+
+	public abstract Long getId();
+
+	public abstract void setId(Long theId);
+
+	public abstract void calculateHashes();
+
+	/**
+	 * Subclasses must implement
+	 */
+	@Override
+	public abstract int hashCode();
+
+	/**
+	 * Subclasses must implement
+	 */
+	@Override
+	public abstract boolean equals(Object obj);
+
+}

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

@@ -31,11 +31,10 @@ import org.hibernate.search.annotations.ContainedIn;
 import org.hibernate.search.annotations.Field;
 
 import javax.persistence.*;
-import java.io.Serializable;
 import java.util.Date;
 
 @MappedSuperclass
-public abstract class BaseResourceIndexedSearchParam implements Serializable {
+public abstract class BaseResourceIndexedSearchParam extends BaseResourceIndex {
 	static final int MAX_SP_NAME = 100;
 	/**
 	 * Don't change this without careful consideration. You will break existing hashes!
@@ -80,7 +79,8 @@ public abstract class BaseResourceIndexedSearchParam implements Serializable {
 		// nothing
 	}
 
-	protected abstract Long getId();
+	@Override
+	public abstract Long getId();
 
 	public String getParamName() {
 		return myParamName;
@@ -129,8 +129,6 @@ public abstract class BaseResourceIndexedSearchParam implements Serializable {
 
 	public abstract IQueryParameterType toQueryParameterType();
 
-	public abstract void calculateHashes();
-
 	public static long calculateHashIdentity(String theResourceType, String theParamName) {
 		return hash(theResourceType, theParamName);
 	}

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

@@ -112,10 +112,16 @@ public class ResourceIndexedSearchParamCoords extends BaseResourceIndexedSearchP
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
+
 	public double getLatitude() {
 		return myLatitude;
 	}
@@ -156,4 +162,5 @@ public class ResourceIndexedSearchParamCoords extends BaseResourceIndexedSearchP
 		b.append("lon", getLongitude());
 		return b.build();
 	}
+
 }

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

@@ -130,10 +130,15 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
 	protected Long getTimeFromDate(Date date) {
 		if (date != null) {
 			return date.getTime();
@@ -212,4 +217,5 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar
 		}
 		return result;
 	}
+
 }

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

@@ -114,10 +114,15 @@ public class ResourceIndexedSearchParamNumber extends BaseResourceIndexedSearchP
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
 	public BigDecimal getValue() {
 		return myValue;
 	}
@@ -155,7 +160,8 @@ public class ResourceIndexedSearchParamNumber extends BaseResourceIndexedSearchP
 		if (!(theParam instanceof NumberParam)) {
 			return false;
 		}
-		NumberParam number = (NumberParam)theParam;
+		NumberParam number = (NumberParam) theParam;
 		return getValue().equals(number.getValue());
 	}
+
 }

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

@@ -166,10 +166,15 @@ public class ResourceIndexedSearchParamQuantity extends BaseResourceIndexedSearc
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
 	public String getSystem() {
 		return mySystem;
 	}
@@ -227,20 +232,12 @@ public class ResourceIndexedSearchParamQuantity extends BaseResourceIndexedSearc
 		return b.build();
 	}
 
-	public static long calculateHashSystemAndUnits(String theResourceType, String theParamName, String theSystem, String theUnits) {
-		return hash(theResourceType, theParamName, theSystem, theUnits);
-	}
-
-	public static long calculateHashUnits(String theResourceType, String theParamName, String theUnits) {
-		return hash(theResourceType, theParamName, theUnits);
-	}
-
 	@Override
 	public boolean matches(IQueryParameterType theParam) {
 		if (!(theParam instanceof QuantityParam)) {
 			return false;
 		}
-		QuantityParam quantity = (QuantityParam)theParam;
+		QuantityParam quantity = (QuantityParam) theParam;
 		boolean retval = false;
 
 		// Only match on system if it wasn't specified
@@ -268,4 +265,13 @@ public class ResourceIndexedSearchParamQuantity extends BaseResourceIndexedSearc
 		return retval;
 	}
 
+	public static long calculateHashSystemAndUnits(String theResourceType, String theParamName, String theSystem, String theUnits) {
+		return hash(theResourceType, theParamName, theSystem, theUnits);
+	}
+
+	public static long calculateHashUnits(String theResourceType, String theParamName, String theUnits) {
+		return hash(theResourceType, theParamName, theUnits);
+	}
+
+
 }

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

@@ -221,10 +221,16 @@ public class ResourceIndexedSearchParamString extends BaseResourceIndexedSearchP
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
+
 	public String getValueExact() {
 		return myValueExact;
 	}

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

@@ -159,6 +159,10 @@ public class ResourceIndexedSearchParamToken extends BaseResourceIndexedSearchPa
 		return myHashSystem;
 	}
 
+	private void setHashSystem(Long theHashSystem) {
+		myHashSystem = theHashSystem;
+	}
+
 	private Long getHashIdentity() {
 		calculateHashes();
 		return myHashIdentity;
@@ -168,10 +172,6 @@ public class ResourceIndexedSearchParamToken extends BaseResourceIndexedSearchPa
 		myHashIdentity = theHashIdentity;
 	}
 
-	private void setHashSystem(Long theHashSystem) {
-		myHashSystem = theHashSystem;
-	}
-
 	Long getHashSystemAndValue() {
 		calculateHashes();
 		return myHashSystemAndValue;
@@ -192,10 +192,15 @@ public class ResourceIndexedSearchParamToken extends BaseResourceIndexedSearchPa
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
 	public String getSystem() {
 		return mySystem;
 	}
@@ -240,24 +245,12 @@ public class ResourceIndexedSearchParamToken extends BaseResourceIndexedSearchPa
 		return b.build();
 	}
 
-	public static long calculateHashSystem(String theResourceType, String theParamName, String theSystem) {
-		return hash(theResourceType, theParamName, trim(theSystem));
-	}
-
-	public static long calculateHashSystemAndValue(String theResourceType, String theParamName, String theSystem, String theValue) {
-		return hash(theResourceType, theParamName, defaultString(trim(theSystem)), trim(theValue));
-	}
-
-	public static long calculateHashValue(String theResourceType, String theParamName, String theValue) {
-		return hash(theResourceType, theParamName, trim(theValue));
-	}
-
 	@Override
 	public boolean matches(IQueryParameterType theParam) {
 		if (!(theParam instanceof TokenParam)) {
 			return false;
 		}
-		TokenParam token = (TokenParam)theParam;
+		TokenParam token = (TokenParam) theParam;
 		boolean retval = false;
 		// Only match on system if it wasn't specified
 		if (token.getSystem() == null || token.getSystem().isEmpty()) {
@@ -276,4 +269,17 @@ public class ResourceIndexedSearchParamToken extends BaseResourceIndexedSearchPa
 		}
 		return retval;
 	}
+
+	public static long calculateHashSystem(String theResourceType, String theParamName, String theSystem) {
+		return hash(theResourceType, theParamName, trim(theSystem));
+	}
+
+	public static long calculateHashSystemAndValue(String theResourceType, String theParamName, String theSystem, String theValue) {
+		return hash(theResourceType, theParamName, defaultString(trim(theSystem)), trim(theValue));
+	}
+
+	public static long calculateHashValue(String theResourceType, String theParamName, String theValue) {
+		return hash(theResourceType, theParamName, trim(theValue));
+	}
+
 }

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

@@ -138,10 +138,16 @@ public class ResourceIndexedSearchParamUri extends BaseResourceIndexedSearchPara
 	}
 
 	@Override
-	protected Long getId() {
+	public Long getId() {
 		return myId;
 	}
 
+	@Override
+	public void setId(Long theId) {
+		myId =theId;
+	}
+
+
 	public String getUri() {
 		return myUri;
 	}
@@ -175,17 +181,18 @@ public class ResourceIndexedSearchParamUri extends BaseResourceIndexedSearchPara
 		return b.toString();
 	}
 
-	public static long calculateHashUri(String theResourceType, String theParamName, String theUri) {
-		return hash(theResourceType, theParamName, theUri);
-	}
-
 	@Override
 	public boolean matches(IQueryParameterType theParam) {
 		if (!(theParam instanceof UriParam)) {
 			return false;
 		}
-		UriParam uri = (UriParam)theParam;
+		UriParam uri = (UriParam) theParam;
 		return getUri().equalsIgnoreCase(uri.getValueNotNull());
 	}
 
+	public static long calculateHashUri(String theResourceType, String theParamName, String theUri) {
+		return hash(theResourceType, theParamName, theUri);
+	}
+
+
 }

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

@@ -27,7 +27,6 @@ import org.hibernate.search.annotations.Field;
 import org.hl7.fhir.instance.model.api.IIdType;
 
 import javax.persistence.*;
-import java.io.Serializable;
 import java.util.Date;
 
 @Entity
@@ -36,11 +35,10 @@ import java.util.Date;
 	@Index(name = "IDX_RL_SRC", columnList = "SRC_RESOURCE_ID"),
 	@Index(name = "IDX_RL_DEST", columnList = "TARGET_RESOURCE_ID")
 })
-public class ResourceLink implements Serializable {
+public class ResourceLink extends BaseResourceIndex {
 
-	private static final long serialVersionUID = 1L;
 	public static final int SRC_PATH_LENGTH = 200;
-
+	private static final long serialVersionUID = 1L;
 	@SequenceGenerator(name = "SEQ_RESLINK_ID", sequenceName = "SEQ_RESLINK_ID")
 	@GeneratedValue(strategy = GenerationType.AUTO, generator = "SEQ_RESLINK_ID")
 	@Id
@@ -126,10 +124,20 @@ public class ResourceLink implements Serializable {
 		return mySourcePath;
 	}
 
+	public void setSourcePath(String theSourcePath) {
+		mySourcePath = theSourcePath;
+	}
+
 	public ResourceTable getSourceResource() {
 		return mySourceResource;
 	}
 
+	public void setSourceResource(ResourceTable theSourceResource) {
+		mySourceResource = theSourceResource;
+		mySourceResourcePid = theSourceResource.getId();
+		mySourceResourceType = theSourceResource.getResourceType();
+	}
+
 	public Long getSourceResourcePid() {
 		return mySourceResourcePid;
 	}
@@ -138,6 +146,13 @@ public class ResourceLink implements Serializable {
 		return myTargetResource;
 	}
 
+	public void setTargetResource(ResourceTable theTargetResource) {
+		Validate.notNull(theTargetResource);
+		myTargetResource = theTargetResource;
+		myTargetResourcePid = theTargetResource.getId();
+		myTargetResourceType = theTargetResource.getResourceType();
+	}
+
 	public Long getTargetResourcePid() {
 		return myTargetResourcePid;
 	}
@@ -146,37 +161,6 @@ public class ResourceLink implements Serializable {
 		return myTargetResourceUrl;
 	}
 
-	public Date getUpdated() {
-		return myUpdated;
-	}
-
-	@Override
-	public int hashCode() {
-		HashCodeBuilder b = new HashCodeBuilder();
-		b.append(mySourcePath);
-		b.append(mySourceResource);
-		b.append(myTargetResourcePid);
-		b.append(myTargetResourceUrl);
-		return b.toHashCode();
-	}
-
-	public void setSourcePath(String theSourcePath) {
-		mySourcePath = theSourcePath;
-	}
-
-	public void setSourceResource(ResourceTable theSourceResource) {
-		mySourceResource = theSourceResource;
-		mySourceResourcePid = theSourceResource.getId();
-		mySourceResourceType = theSourceResource.getResourceType();
-	}
-
-	public void setTargetResource(ResourceTable theTargetResource) {
-		Validate.notNull(theTargetResource);
-		myTargetResource = theTargetResource;
-		myTargetResourcePid = theTargetResource.getId();
-		myTargetResourceType = theTargetResource.getResourceType();
-	}
-
 	public void setTargetResourceUrl(IIdType theTargetResourceUrl) {
 		Validate.isTrue(theTargetResourceUrl.hasBaseUrl());
 		Validate.isTrue(theTargetResourceUrl.hasResourceType());
@@ -194,10 +178,39 @@ public class ResourceLink implements Serializable {
 		myTargetResourceUrl = theTargetResourceUrl.getValue();
 	}
 
+	public Date getUpdated() {
+		return myUpdated;
+	}
+
 	public void setUpdated(Date theUpdated) {
 		myUpdated = theUpdated;
 	}
 
+	@Override
+	public Long getId() {
+		return myId;
+	}
+
+	@Override
+	public void setId(Long theId) {
+		myId = theId;
+	}
+
+	@Override
+	public void calculateHashes() {
+		// nothing right now
+	}
+
+	@Override
+	public int hashCode() {
+		HashCodeBuilder b = new HashCodeBuilder();
+		b.append(mySourcePath);
+		b.append(mySourceResource);
+		b.append(myTargetResourcePid);
+		b.append(myTargetResourceUrl);
+		return b.toHashCode();
+	}
+
 	@Override
 	public String toString() {
 		StringBuilder b = new StringBuilder();

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

@@ -32,10 +32,8 @@ import org.hibernate.search.annotations.*;
 import javax.persistence.Index;
 import javax.persistence.*;
 import java.io.Serializable;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collection;
+import java.util.stream.Collectors;
-import java.util.HashSet;
-import java.util.Set;
 
 import static org.apache.commons.lang3.StringUtils.defaultString;
 
@@ -177,10 +175,27 @@ public class ResourceTable extends BaseHasResource implements Serializable {
 	@OptimisticLock(excluded = true)
 	private Collection<ResourceIndexedCompositeStringUnique> myParamsCompositeStringUnique;
 
-	@IndexedEmbedded
 	@OneToMany(mappedBy = "mySourceResource", cascade = {}, fetch = FetchType.LAZY, orphanRemoval = false)
 	@OptimisticLock(excluded = true)
 	private Collection<ResourceLink> myResourceLinks;
+
+	/**
+	 * This is a clone of {@link #myResourceLinks} but without the hibernate annotations.
+	 * Before we persist we copy the contents of {@link #myResourceLinks} into this field. We
+	 * have this separate because that way we can only populate this field if
+	 * {@link #myHasLinks} is true, meaning that there are actually resource links present
+	 * right now. This avoids Hibernate Search triggering a select on the resource link
+	 * table.
+	 *
+	 * This field is used by FulltextSearchSvcImpl
+	 *
+	 * You can test that any changes don't cause extra queries by running
+	 * FhirResourceDaoR4QueryCountTest
+	 */
+	@Field
+	@Transient
+	private String myResourceLinksField;
+
 	@OneToMany(mappedBy = "myTargetResource", cascade = {}, fetch = FetchType.LAZY, orphanRemoval = false)
 	@OptimisticLock(excluded = true)
 	private Collection<ResourceLink> myResourceLinksAsTarget;
@@ -589,4 +604,22 @@ public class ResourceTable extends BaseHasResource implements Serializable {
 		return b.build();
 	}
 
+	@PrePersist
+	@PreUpdate
+	public void preSave() {
+		if (myHasLinks && myResourceLinks != null) {
+			myResourceLinksField = getResourceLinks()
+				.stream()
+				.map(t->{
+					Long retVal = t.getTargetResourcePid();
+					return retVal;
+				})
+				.filter(Objects::nonNull)
+				.map(t->t.toString())
+				.collect(Collectors.joining(" "));
+		} else {
+			myResourceLinksField = null;
+		}
+	}
+
 }

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

@@ -208,8 +208,8 @@ public final class ResourceIndexedSearchParams {
 
 
 
-	public void calculateHashes(Collection<? extends BaseResourceIndexedSearchParam> theStringParams) {
+	public void calculateHashes(Collection<? extends BaseResourceIndex> theStringParams) {
-		for (BaseResourceIndexedSearchParam next : theStringParams) {
+		for (BaseResourceIndex next : theStringParams) {
 			next.calculateHashes();
 		}
 	}
@@ -353,6 +353,7 @@ public final class ResourceIndexedSearchParams {
 						case COMPOSITE:
 						case HAS:
 						case REFERENCE:
+						case SPECIAL:
 						default:
 							continue;
 					}

src/changes/changes.xml

@@ -122,6 +122,12 @@
 				adjust the most recent version to
 				account for this.
 			</action>
+			<action type="add">
+				When updating existing resources, the JPA server will now attempt to reuse/update
+				rows in the index tables if one row is being removed and one row is being added (e.g.
+				because a Patient's name is changing from "A" to "B"). This has the net effect
+				of reducing the number
+			</action>
 		</release>
 		<release version="3.6.0" date="2018-11-12" description="Food">
 			<action type="add">