honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Change chain sort syntax for MS SQL (#5917) 

* Change sort type on chains

* Change sort type on chains

* Test for MS SQL

* Comments
This commit is contained in:
Michael Buckley 2024-05-08 23:28:26 -04:00 committed by GitHub
parent 9021e7e765
commit 6d6c140986
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 110 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/5917-chain-sort-mssql.yaml Normal file
View File

@ -0,0 +1,4 @@
---
type: fix
issue: 5917
title: "Fix chained sorts on strings when using MS Sql"

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

@ -353,26 +353,39 @@ public class QueryStack {
throw new InvalidRequestException(Msg.code(2289) + msg);
}
BaseSearchParamPredicateBuilder chainedPredicateBuilder;
DbColumn[] sortColumn;
// add a left-outer join to a predicate for the target type, then sort on value columns(s).
switch (targetSearchParameter.getParamType()) {
case STRING:
StringPredicateBuilder stringPredicateBuilder = mySqlBuilder.createStringPredicateBuilder();
sortColumn = new DbColumn[] {stringPredicateBuilder.getColumnValueNormalized()};
chainedPredicateBuilder = stringPredicateBuilder;
break;
addSortCustomJoin(
resourceLinkPredicateBuilder.getColumnTargetResourceId(),
stringPredicateBuilder,
stringPredicateBuilder.createHashIdentityPredicate(targetType, theChain));
mySqlBuilder.addSortString(
stringPredicateBuilder.getColumnValueNormalized(), theAscending, myUseAggregate);
return;
case TOKEN:
TokenPredicateBuilder tokenPredicateBuilder = mySqlBuilder.createTokenPredicateBuilder();
sortColumn =
new DbColumn[] {tokenPredicateBuilder.getColumnSystem(), tokenPredicateBuilder.getColumnValue()
};
chainedPredicateBuilder = tokenPredicateBuilder;
break;
addSortCustomJoin(
resourceLinkPredicateBuilder.getColumnTargetResourceId(),
tokenPredicateBuilder,
tokenPredicateBuilder.createHashIdentityPredicate(targetType, theChain));
mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnSystem(), theAscending, myUseAggregate);
mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnValue(), theAscending, myUseAggregate);
return;
case DATE:
DatePredicateBuilder datePredicateBuilder = mySqlBuilder.createDatePredicateBuilder();
sortColumn = new DbColumn[] {datePredicateBuilder.getColumnValueLow()};
chainedPredicateBuilder = datePredicateBuilder;
break;
addSortCustomJoin(
resourceLinkPredicateBuilder.getColumnTargetResourceId(),
datePredicateBuilder,
datePredicateBuilder.createHashIdentityPredicate(targetType, theChain));
mySqlBuilder.addSortDate(datePredicateBuilder.getColumnValueLow(), theAscending, myUseAggregate);
return;
/*
* Note that many of the options below aren't implemented because they
@ -417,16 +430,6 @@ public class QueryStack {
+ theParamName + "." + theChain + " as this parameter. Can not sort on chains of target type: "
+ targetSearchParameter.getParamType().name());
}
addSortCustomJoin(resourceLinkPredicateBuilder.getColumnTargetResourceId(), chainedPredicateBuilder, null);
// Support chained sort - when there is no data to sort, the outer join will produce null columns. We need to
// include those too.
Condition predicate = chainedPredicateBuilder.createHashIdentityOrNullPredicate(targetType, theChain);
mySqlBuilder.addPredicate(predicate);
for (DbColumn next : sortColumn) {
mySqlBuilder.addSortNumeric(next, theAscending, myUseAggregate);
}
}
public void addSortOnString(String theResourceName, String theParamName, boolean theAscending) {

11
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/BaseSearchParamPredicateBuilder.java
View File

@ -27,7 +27,6 @@ import ca.uhn.fhir.jpa.util.QueryParameterUtils;
import com.healthmarketscience.sqlbuilder.BinaryCondition;
import com.healthmarketscience.sqlbuilder.ComboCondition;
import com.healthmarketscience.sqlbuilder.Condition;
import com.healthmarketscience.sqlbuilder.Conditions;
import com.healthmarketscience.sqlbuilder.NotCondition;
import com.healthmarketscience.sqlbuilder.SelectQuery;
import com.healthmarketscience.sqlbuilder.UnaryCondition;
@ -97,16 +96,6 @@ public abstract class BaseSearchParamPredicateBuilder extends BaseJoiningPredica
return BinaryCondition.equalTo(myColumnHashIdentity, hashIdentityVal);
}
@Nonnull
public Condition createHashIdentityOrNullPredicate(String theResourceType, String theParamName) {
final long hashIdentity = BaseResourceIndexedSearchParam.calculateHashIdentity(
getPartitionSettings(), getRequestPartitionId(), theResourceType, theParamName);
final String hashIdentityVal = generatePlaceholder(hashIdentity);
return Conditions.or(
BinaryCondition.equalTo(myColumnHashIdentity, hashIdentityVal),
Conditions.isNull(myColumnHashIdentity));
}
public Condition createPredicateParamMissingForNonReference(
String theResourceName, String theParamName, Boolean theMissing, RequestPartitionId theRequestPartitionId) {
ComboCondition condition = ComboCondition.and(

6
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QuerySandboxTest.java → hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QuerySandbox.java
View File

@ -54,10 +54,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
@TestExecutionListeners(listeners = {
DependencyInjectionTestExecutionListener.class
, FhirResourceDaoR4QuerySandboxTest.TestDirtiesContextTestExecutionListener.class
, FhirResourceDaoR4QuerySandbox.TestDirtiesContextTestExecutionListener.class
})
public class FhirResourceDaoR4QuerySandboxTest extends BaseJpaTest {
private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4QuerySandboxTest.class);
public class FhirResourceDaoR4QuerySandbox extends BaseJpaTest {
private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4QuerySandbox.class);
@Autowired
PlatformTransactionManager myTxManager;

60
hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/UpliftedRefchainsAndChainedSortingR5Test.java
View File

@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r5;
import ca.uhn.fhir.context.RuntimeSearchParam;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.dao.TestDaoSearch;
import ca.uhn.fhir.jpa.model.entity.StorageSettings;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.test.config.TestHSearchAddInConfig;
@ -12,6 +13,8 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.util.BundleBuilder;
import ca.uhn.fhir.util.HapiExtensions;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r5.model.Composition;
import org.hl7.fhir.r5.model.IdType;
import org.hl7.fhir.r5.model.Bundle;
@ -29,6 +32,7 @@ import org.hl7.fhir.r5.model.SearchParameter;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.ContextConfiguration;
import jakarta.annotation.Nonnull;
@ -41,9 +45,10 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@ContextConfiguration(classes = TestHSearchAddInConfig.NoFT.class)
@ContextConfiguration(classes = { TestHSearchAddInConfig.NoFT.class, TestDaoSearch.Config.class })
@SuppressWarnings({"Duplicates"})
public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
public static final String PRACTITIONER_PR1 = "Practitioner/PR1";
@ -54,6 +59,8 @@ public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
public static final String ENCOUNTER_E2 = "Encounter/E2";
public static final String ENCOUNTER_E3 = "Encounter/E3";
public static final String ORGANIZATION_O1 = "Organization/O1";
@Autowired
protected TestDaoSearch myTestDaoSearch;
@Override
@BeforeEach
@ -724,7 +731,7 @@ public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
String querySql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false);
assertEquals(1, countMatches(querySql, "HFJ_SPIDX_STRING"), querySql);
assertEquals(0, countMatches(querySql, "HASH_NORM_PREFIX"), querySql);
assertEquals(2, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HFJ_RES_LINK"), querySql);
}
@ -760,7 +767,7 @@ public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
String querySql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false);
assertEquals(2, countMatches(querySql, "HFJ_SPIDX_STRING"), querySql);
assertEquals(1, countMatches(querySql, "HASH_NORM_PREFIX"), querySql);
assertEquals(2, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(2, countMatches(querySql, "HFJ_RES_LINK"), querySql);
}
@ -794,7 +801,7 @@ public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
assertEquals(2, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
String querySql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false);
assertEquals(1, countMatches(querySql, "HFJ_SPIDX_TOKEN"), querySql);
assertEquals(2, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HFJ_RES_LINK"), querySql);
}
@ -828,7 +835,7 @@ public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
assertEquals(2, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
String querySql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false);
assertEquals(1, countMatches(querySql, "HFJ_SPIDX_DATE"), querySql);
assertEquals(2, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HFJ_RES_LINK"), querySql);
}
@ -863,10 +870,51 @@ public class UpliftedRefchainsAndChainedSortingR5Test extends BaseJpaR5Test {
String querySql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false);
assertEquals(1, countMatches(querySql, "HFJ_SPIDX_STRING"), querySql);
assertEquals(0, countMatches(querySql, "HASH_NORM_PREFIX"), querySql);
assertEquals(2, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HASH_IDENTITY"), querySql);
assertEquals(1, countMatches(querySql, "HFJ_RES_LINK"), querySql);
}
@Test
void testChainedSortWithNulls() {
final IIdType practitionerId1 = createPractitioner(withFamily("Chan"));
final IIdType practitionerId2 = createPractitioner(withFamily("Jones"));
final String id1 = createPatient(withFamily("Smithy")).getIdPart();
final String id2 = createPatient(withFamily("Smithwick"),
withReference("generalPractitioner", practitionerId2)).getIdPart();
final String id3 = createPatient(
withFamily("Smith"),
withReference("generalPractitioner", practitionerId1)).getIdPart();
final IBundleProvider iBundleProvider = myTestDaoSearch.searchForBundleProvider("Patient?_total=ACCURATE&_sort=Practitioner:general-practitioner.family");
final List<IBaseResource> allResources = iBundleProvider.getAllResources();
assertEquals(3, iBundleProvider.size());
assertEquals(3, allResources.size());
final List<String> actualIds = allResources.stream().map(IBaseResource::getIdElement).map(IIdType::getIdPart).toList();
assertTrue(actualIds.containsAll(List.of(id1, id2, id3)));
}
@Test
void testChainedReverseStringSort() {
final IIdType practitionerId = createPractitioner(withFamily("Jones"));
final String id1 = createPatient(withFamily("Smithy")).getIdPart();
final String id2 = createPatient(withFamily("Smithwick")).getIdPart();
final String id3 = createPatient(
withFamily("Smith"),
withReference("generalPractitioner", practitionerId)).getIdPart();
final IBundleProvider iBundleProvider = myTestDaoSearch.searchForBundleProvider("Patient?_total=ACCURATE&_sort=-Practitioner:general-practitioner.family");
assertEquals(3, iBundleProvider.size());
final List<IBaseResource> allResources = iBundleProvider.getAllResources();
final List<String> actualIds = allResources.stream().map(IBaseResource::getIdElement).map(IIdType::getIdPart).toList();
assertTrue(actualIds.containsAll(List.of(id3, id2, id1)));
}
/**
* Observation:focus is a Reference(Any) so it can't be used in a sort chain because
* this would be horribly, horribly inefficient.

47
hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/database/BaseDatabaseVerificationIT.java
View File

@ -1,38 +1,24 @@
package ca.uhn.fhir.jpa.dao.r5.database;
import ca.uhn.fhir.batch2.jobs.export.BulkDataExportProvider;
import ca.uhn.fhir.batch2.jobs.expunge.DeleteExpungeProvider;
import ca.uhn.fhir.batch2.jobs.reindex.ReindexProvider;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoPatient;
import ca.uhn.fhir.jpa.api.dao.PatientEverythingParameters;
import ca.uhn.fhir.jpa.dao.TestDaoSearch;
import ca.uhn.fhir.jpa.embedded.JpaEmbeddedDatabase;
import ca.uhn.fhir.jpa.fql.provider.HfqlRestProvider;
import ca.uhn.fhir.jpa.graphql.GraphQLProvider;
import ca.uhn.fhir.jpa.migrate.HapiMigrationStorageSvc;
import ca.uhn.fhir.jpa.migrate.MigrationTaskList;
import ca.uhn.fhir.jpa.migrate.SchemaMigrator;
import ca.uhn.fhir.jpa.migrate.dao.HapiMigrationDao;
import ca.uhn.fhir.jpa.migrate.tasks.HapiFhirJpaMigrationTasks;
import ca.uhn.fhir.jpa.provider.DiffProvider;
import ca.uhn.fhir.jpa.provider.JpaCapabilityStatementProvider;
import ca.uhn.fhir.jpa.provider.ProcessMessageProvider;
import ca.uhn.fhir.jpa.provider.SubscriptionTriggeringProvider;
import ca.uhn.fhir.jpa.provider.TerminologyUploaderProvider;
import ca.uhn.fhir.jpa.provider.ValueSetOperationProvider;
import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.test.BaseJpaTest;
import ca.uhn.fhir.jpa.test.config.TestR5Config;
import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.SortSpec;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.interceptor.CorsInterceptor;
import ca.uhn.fhir.rest.server.provider.ResourceProviderFactory;
import ca.uhn.fhir.test.utilities.ITestDataBuilder;
import ca.uhn.fhir.test.utilities.server.RestfulServerConfigurerExtension;
@ -44,7 +30,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r5.model.Bundle;
import org.hl7.fhir.r5.model.IdType;
import org.hl7.fhir.r5.model.IntegerType;
import org.hl7.fhir.r5.model.Parameters;
import org.hl7.fhir.r5.model.Patient;
import org.junit.jupiter.api.Test;
@ -55,7 +40,6 @@ import org.junit.jupiter.params.provider.ValueSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.envers.repository.support.EnversRevisionRepositoryFactoryBean;
@ -63,10 +47,8 @@ import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.web.cors.CorsConfiguration;
import javax.sql.DataSource;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Properties;
@ -80,7 +62,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
@ExtendWith(SpringExtension.class)
@EnableJpaRepositories(repositoryFactoryBeanClass = EnversRevisionRepositoryFactoryBean.class)
@ContextConfiguration(classes = {BaseDatabaseVerificationIT.TestConfig.class})
@ContextConfiguration(classes = {BaseDatabaseVerificationIT.TestConfig.class, TestDaoSearch.Config.class})
public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements ITestDataBuilder {
private static final Logger ourLog = LoggerFactory.getLogger(BaseDatabaseVerificationIT.class);
private static final String MIGRATION_TABLENAME = "MIGRATIONS";
@ -109,6 +91,9 @@ public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements
@Autowired
private DatabaseBackedPagingProvider myPagingProvider;
@Autowired
TestDaoSearch myTestDaoSearch;
@RegisterExtension
protected RestfulServerExtension myServer = new RestfulServerExtension(FhirContext.forR5Cached());
@ -175,6 +160,20 @@ public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements
assertThat(values.toString(), values, containsInAnyOrder(expectedIds.toArray(new String[0])));
}
@Test
void testChainedSort() {
// given
// when
SearchParameterMap map = SearchParameterMap
.newSynchronous()
.setSort(new SortSpec("Practitioner:general-practitioner.family"));
myCaptureQueriesListener.clear();
myPatientDao.search(map, mySrd);
}
@Configuration
@ -222,8 +221,8 @@ public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements
}
public static class JpaDatabaseContextConfigParamObject {
private JpaEmbeddedDatabase myJpaEmbeddedDatabase;
private String myDialect;
final JpaEmbeddedDatabase myJpaEmbeddedDatabase;
final String myDialect;
public JpaDatabaseContextConfigParamObject(JpaEmbeddedDatabase theJpaEmbeddedDatabase, String theDialect) {
myJpaEmbeddedDatabase = theJpaEmbeddedDatabase;