diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2262-fix-nullable-column-sorting-in-certain-dbs.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2262-fix-nullable-column-sorting-in-certain-dbs.yaml new file mode 100644 index 00000000000..1e1a852ac85 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2262-fix-nullable-column-sorting-in-certain-dbs.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 2262 +title: "Sorting of search results was not working for MySQL, MSSQL and MariaDB due to recent changes made to handle sorting + of nullable columns. This has now been fixed." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java index 3ea0223a50f..1a23e07da81 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java @@ -159,7 +159,7 @@ public class QueryStack { Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName); mySqlBuilder.addPredicate(hashIdentityPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnValueLow(), theAscending); + mySqlBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending); } public void addSortOnLastUpdated(boolean theAscending) { @@ -170,7 +170,7 @@ public class QueryStack { } else { resourceTablePredicateBuilder = mySqlBuilder.addResourceTablePredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); } - mySqlBuilder.addSort(resourceTablePredicateBuilder.getColumnLastUpdated(), theAscending); + mySqlBuilder.addSortDate(resourceTablePredicateBuilder.getColumnLastUpdated(), theAscending); } @@ -180,7 +180,7 @@ public class QueryStack { Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName); mySqlBuilder.addPredicate(hashIdentityPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnValue(), theAscending); } public void addSortOnQuantity(String theResourceName, String theParamName, boolean theAscending) { @@ -189,18 +189,18 @@ public class QueryStack { Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName); mySqlBuilder.addPredicate(hashIdentityPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnValue(), theAscending); } public void addSortOnResourceId(boolean theAscending) { BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); ForcedIdPredicateBuilder sortPredicateBuilder = mySqlBuilder.addForcedIdPredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); if (!theAscending) { - mySqlBuilder.addSort(sortPredicateBuilder.getColumnForcedId(), false, OrderObject.NullOrder.FIRST); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), false, OrderObject.NullOrder.FIRST); } else { - mySqlBuilder.addSort(sortPredicateBuilder.getColumnForcedId(), true); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), true); } - mySqlBuilder.addSort(firstPredicateBuilder.getResourceIdColumn(), theAscending); + mySqlBuilder.addSortNumeric(firstPredicateBuilder.getResourceIdColumn(), theAscending); } @@ -210,7 +210,7 @@ public class QueryStack { Condition pathPredicate = sortPredicateBuilder.createPredicateSourcePaths(theResourceName, theParamName); mySqlBuilder.addPredicate(pathPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnTargetResourceId(), theAscending); + mySqlBuilder.addSortNumeric(sortPredicateBuilder.getColumnTargetResourceId(), theAscending); } @@ -220,7 +220,7 @@ public class QueryStack { Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName); mySqlBuilder.addPredicate(hashIdentityPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnValueNormalized(), theAscending); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending); } public void addSortOnToken(String theResourceName, String theParamName, boolean theAscending) { @@ -229,8 +229,8 @@ public class QueryStack { Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName); mySqlBuilder.addPredicate(hashIdentityPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnSystem(), theAscending); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnSystem(), theAscending); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValue(), theAscending); } public void addSortOnUri(String theResourceName, String theParamName, boolean theAscending) { @@ -239,7 +239,7 @@ public class QueryStack { Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName); mySqlBuilder.addPredicate(hashIdentityPredicate); - mySqlBuilder.addSort(sortPredicateBuilder.getColumnValue(), theAscending); + mySqlBuilder.addSortString(sortPredicateBuilder.getColumnValue(), theAscending); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java index 44ecad70e8f..667539ca770 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java @@ -95,6 +95,8 @@ public class SearchQueryBuilder { private ResourceTablePredicateBuilder myResourceTableRoot; private boolean myHaveAtLeastOnePredicate; private BaseJoiningPredicateBuilder myFirstPredicateBuilder; + private boolean dialectIsMsSql; + private boolean dialectIsMySql; /** * Constructor @@ -115,6 +117,13 @@ public class SearchQueryBuilder { mySqlBuilderFactory = theSqlBuilderFactory; myCountQuery = theCountQuery; myDialect = theDialect; + if (myDialect instanceof org.hibernate.dialect.MySQLDialect){ + dialectIsMySql = true; + } + if (myDialect instanceof org.hibernate.dialect.SQLServerDialect){ + dialectIsMsSql = true; + } + mySpec = new DbSpec(); mySchema = mySpec.addDefaultSchema(); @@ -550,12 +559,92 @@ public class SearchQueryBuilder { return myHaveAtLeastOnePredicate; } - public void addSort(DbColumn theColumnValueNormalized, boolean theAscending) { + public void addSortString(DbColumn theColumnValueNormalized, boolean theAscending) { OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; - addSort(theColumnValueNormalized, theAscending, nullOrder); + addSortString(theColumnValueNormalized, theAscending, nullOrder); } - public void addSort(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + public void addSortNumeric(DbColumn theColumnValueNormalized, boolean theAscending) { + OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; + addSortNumeric(theColumnValueNormalized, theAscending, nullOrder); + } + + public void addSortDate(DbColumn theColumnValueNormalized, boolean theAscending) { + OrderObject.NullOrder nullOrder = OrderObject.NullOrder.LAST; + addSortDate(theColumnValueNormalized, theAscending, nullOrder); + } + + public void addSortString(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + if ((dialectIsMySql || dialectIsMsSql)) { + // MariaDB, MySQL and MSSQL do not support "NULLS FIRST" and "NULLS LAST" syntax. + String direction = theTheAscending ? " ASC" : " DESC"; + String sortColumnName = theTheColumnValueNormalized.getTable().getAlias() + "." + theTheColumnValueNormalized.getName(); + final StringBuilder sortColumnNameBuilder = new StringBuilder(); + // The following block has been commented out for performance. + // Uncomment if NullOrder is needed for MariaDB, MySQL or MSSQL + /* + // Null values are always treated as less than non-null values. + if ((theTheAscending && theNullOrder == OrderObject.NullOrder.LAST) + || (!theTheAscending && theNullOrder == OrderObject.NullOrder.FIRST)) { + // In this case, precede the "order by" column with a case statement that returns + // 1 for null and 0 non-null so that nulls will be sorted as greater than non-nulls. + sortColumnNameBuilder.append( "CASE WHEN " ).append( sortColumnName ).append( " IS NULL THEN 1 ELSE 0 END" ).append(direction).append(", "); + } + */ + sortColumnNameBuilder.append(sortColumnName).append(direction); + mySelect.addCustomOrderings(sortColumnNameBuilder.toString()); + } else { + addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder); + } + } + + public void addSortNumeric(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + if ((dialectIsMySql || dialectIsMsSql)) { + // MariaDB, MySQL and MSSQL do not support "NULLS FIRST" and "NULLS LAST" syntax. + // Null values are always treated as less than non-null values. + // As such special handling is required here. + String direction; + String sortColumnName = theTheColumnValueNormalized.getTable().getAlias() + "." + theTheColumnValueNormalized.getName(); + if ((theTheAscending && theNullOrder == OrderObject.NullOrder.LAST) + || (!theTheAscending && theNullOrder == OrderObject.NullOrder.FIRST)) { + // Negating the numeric column value and reversing the sort order will ensure that the rows appear + // in the correct order with nulls appearing first or last as needed. + direction = theTheAscending ? " DESC" : " ASC"; + sortColumnName = "-" + sortColumnName; + } else { + direction = theTheAscending ? " ASC" : " DESC"; + } + mySelect.addCustomOrderings(sortColumnName + direction); + } else { + addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder); + } + } + + public void addSortDate(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { + if ((dialectIsMySql || dialectIsMsSql)) { + // MariaDB, MySQL and MSSQL do not support "NULLS FIRST" and "NULLS LAST" syntax. + String direction = theTheAscending ? " ASC" : " DESC"; + String sortColumnName = theTheColumnValueNormalized.getTable().getAlias() + "." + theTheColumnValueNormalized.getName(); + final StringBuilder sortColumnNameBuilder = new StringBuilder(); + // The following block has been commented out for performance. + // Uncomment if NullOrder is needed for MariaDB, MySQL or MSSQL + /* + // Null values are always treated as less than non-null values. + if ((theTheAscending && theNullOrder == OrderObject.NullOrder.LAST) + || (!theTheAscending && theNullOrder == OrderObject.NullOrder.FIRST)) { + // In this case, precede the "order by" column with a case statement that returns + // 1 for null and 0 non-null so that nulls will be sorted as greater than non-nulls. + sortColumnNameBuilder.append( "CASE WHEN " ).append( sortColumnName ).append( " IS NULL THEN 1 ELSE 0 END" ).append(direction).append(", "); + } + */ + sortColumnNameBuilder.append(sortColumnName).append(direction); + mySelect.addCustomOrderings(sortColumnNameBuilder.toString()); + } else { + addSort(theTheColumnValueNormalized, theTheAscending, theNullOrder); + } + } + + private void addSort(DbColumn theTheColumnValueNormalized, boolean theTheAscending, OrderObject.NullOrder theNullOrder) { OrderObject.Dir direction = theTheAscending ? OrderObject.Dir.ASCENDING : OrderObject.Dir.DESCENDING; OrderObject orderObject = new OrderObject(direction, theTheColumnValueNormalized); orderObject.setNullOrder(theNullOrder); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderMySqlTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderMySqlTest.java new file mode 100644 index 00000000000..a040ea1d3da --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderMySqlTest.java @@ -0,0 +1,185 @@ +package ca.uhn.fhir.jpa.search.builder.sql; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.config.HibernateDialectProvider; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.entity.ModelConfig; +import ca.uhn.fhir.jpa.search.builder.predicate.BaseJoiningPredicateBuilder; +import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder; +import ca.uhn.fhir.jpa.search.builder.predicate.ResourceTablePredicateBuilder; +import ca.uhn.fhir.jpa.search.builder.predicate.StringPredicateBuilder; +import com.healthmarketscience.sqlbuilder.Condition; +import com.healthmarketscience.sqlbuilder.OrderObject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +public class SearchQueryBuilderMySqlTest { + + @Mock + private SqlObjectFactory mySqlObjectFactory; + @Mock + private HibernateDialectProvider myHibernateDialectProvider; + + private final FhirContext myFhirContext = FhirContext.forR4(); + + @BeforeEach + public void beforeInitMocks() { + MockitoAnnotations.initMocks(this); + when(myHibernateDialectProvider.getDialect()).thenReturn(new org.hibernate.dialect.MySQL57Dialect()); + } + + private SearchQueryBuilder createSearchQueryBuilder() { + return new SearchQueryBuilder(myFhirContext, new ModelConfig(), new PartitionSettings(), RequestPartitionId.allPartitions(), "Patient", mySqlObjectFactory, myHibernateDialectProvider, false); + } + + @Test + public void testAddSortNumericNoNullOrder() { + GeneratedSql generatedSql = buildSqlWithNumericSort(true,null); + assertTrue(generatedSql.getSql().endsWith("ORDER BY -t1.SP_VALUE_LOW DESC limit ?")); + + generatedSql = buildSqlWithNumericSort(false,null); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW DESC limit ?")); + + } + + private GeneratedSql buildSqlWithNumericSort(Boolean theAscending, OrderObject.NullOrder theNullOrder) { + SearchQueryBuilder searchQueryBuilder = createSearchQueryBuilder(); + when(mySqlObjectFactory.resourceTable(any())).thenReturn(new ResourceTablePredicateBuilder(searchQueryBuilder)); + when(mySqlObjectFactory.dateIndexTable(any())).thenReturn(new DatePredicateBuilder(searchQueryBuilder)); + + BaseJoiningPredicateBuilder firstPredicateBuilder = searchQueryBuilder.getOrCreateFirstPredicateBuilder(); + DatePredicateBuilder sortPredicateBuilder = searchQueryBuilder.addDatePredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); + + Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate("MolecularSequence", "variant-start"); + searchQueryBuilder.addPredicate(hashIdentityPredicate); + if (theNullOrder == null) { + searchQueryBuilder.addSortNumeric(sortPredicateBuilder.getColumnValueLow(), theAscending); + } else { + searchQueryBuilder.addSortNumeric(sortPredicateBuilder.getColumnValueLow(), theAscending, theNullOrder); + } + + return searchQueryBuilder.generate(0,500); + + } + + @Test + public void testAddSortNumericWithNullOrder() { + GeneratedSql generatedSql = buildSqlWithNumericSort(true, OrderObject.NullOrder.FIRST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW ASC limit ?")); + + generatedSql = buildSqlWithNumericSort(false, OrderObject.NullOrder.FIRST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY -t1.SP_VALUE_LOW ASC limit ?")); + + generatedSql = buildSqlWithNumericSort(true, OrderObject.NullOrder.LAST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY -t1.SP_VALUE_LOW DESC limit ?")); + + generatedSql = buildSqlWithNumericSort(false, OrderObject.NullOrder.LAST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW DESC limit ?")); + + } + + @Test + public void testAddSortStringNoNullOrder() { + GeneratedSql generatedSql = buildSqlWithStringSort(true,null); +// assertTrue(generatedSql.getSql().endsWith("ORDER BY CASE WHEN t1.SP_VALUE_NORMALIZED IS NULL THEN 1 ELSE 0 END ASC, t1.SP_VALUE_NORMALIZED ASC limit ?")); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_NORMALIZED ASC limit ?")); + + generatedSql = buildSqlWithStringSort(false,null); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_NORMALIZED DESC limit ?")); + + } + + private GeneratedSql buildSqlWithStringSort(Boolean theAscending, OrderObject.NullOrder theNullOrder) { + SearchQueryBuilder searchQueryBuilder = createSearchQueryBuilder(); + when(mySqlObjectFactory.resourceTable(any())).thenReturn(new ResourceTablePredicateBuilder(searchQueryBuilder)); + when(mySqlObjectFactory.stringIndexTable(any())).thenReturn(new StringPredicateBuilder(searchQueryBuilder)); + + BaseJoiningPredicateBuilder firstPredicateBuilder = searchQueryBuilder.getOrCreateFirstPredicateBuilder(); + StringPredicateBuilder sortPredicateBuilder = searchQueryBuilder.addStringPredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); + + Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate("patient", "family"); + searchQueryBuilder.addPredicate(hashIdentityPredicate); + if (theNullOrder == null) { + searchQueryBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending); + } else { + searchQueryBuilder.addSortString(sortPredicateBuilder.getColumnValueNormalized(), theAscending, theNullOrder); + } + + return searchQueryBuilder.generate(0,500); + + } + + @Test + public void testAddSortStringWithNullOrder() { + GeneratedSql generatedSql = buildSqlWithStringSort(true, OrderObject.NullOrder.FIRST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_NORMALIZED ASC limit ?")); + + generatedSql = buildSqlWithStringSort(false, OrderObject.NullOrder.FIRST); +// assertTrue(generatedSql.getSql().endsWith("ORDER BY CASE WHEN t1.SP_VALUE_NORMALIZED IS NULL THEN 1 ELSE 0 END DESC, t1.SP_VALUE_NORMALIZED DESC limit ?")); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_NORMALIZED DESC limit ?")); + + generatedSql = buildSqlWithStringSort(true, OrderObject.NullOrder.LAST); +// assertTrue(generatedSql.getSql().endsWith("ORDER BY CASE WHEN t1.SP_VALUE_NORMALIZED IS NULL THEN 1 ELSE 0 END ASC, t1.SP_VALUE_NORMALIZED ASC limit ?")); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_NORMALIZED ASC limit ?")); + + generatedSql = buildSqlWithStringSort(false, OrderObject.NullOrder.LAST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_NORMALIZED DESC limit ?")); + + } + + @Test + public void testAddSortDateNoNullOrder() { + GeneratedSql generatedSql = buildSqlWithDateSort(true,null); +// assertTrue(generatedSql.getSql().endsWith("ORDER BY CASE WHEN t1.SP_VALUE_LOW IS NULL THEN 1 ELSE 0 END ASC, t1.SP_VALUE_LOW ASC limit ?")); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW ASC limit ?")); + + generatedSql = buildSqlWithDateSort(false,null); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW DESC limit ?")); + + } + + private GeneratedSql buildSqlWithDateSort(Boolean theAscending, OrderObject.NullOrder theNullOrder) { + SearchQueryBuilder searchQueryBuilder = createSearchQueryBuilder(); + when(mySqlObjectFactory.resourceTable(any())).thenReturn(new ResourceTablePredicateBuilder(searchQueryBuilder)); + when(mySqlObjectFactory.dateIndexTable(any())).thenReturn(new DatePredicateBuilder(searchQueryBuilder)); + + BaseJoiningPredicateBuilder firstPredicateBuilder = searchQueryBuilder.getOrCreateFirstPredicateBuilder(); + DatePredicateBuilder sortPredicateBuilder = searchQueryBuilder.addDatePredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); + + Condition hashIdentityPredicate = sortPredicateBuilder.createHashIdentityPredicate("patient", "birthdate"); + searchQueryBuilder.addPredicate(hashIdentityPredicate); + if (theNullOrder == null) { + searchQueryBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending); + } else { + searchQueryBuilder.addSortDate(sortPredicateBuilder.getColumnValueLow(), theAscending, theNullOrder); + } + + return searchQueryBuilder.generate(0,500); + + } + + @Test + public void testAddSortDateWithNullOrder() { + GeneratedSql generatedSql = buildSqlWithDateSort(true, OrderObject.NullOrder.FIRST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW ASC limit ?")); + + generatedSql = buildSqlWithDateSort(false, OrderObject.NullOrder.FIRST); +// assertTrue(generatedSql.getSql().endsWith("ORDER BY CASE WHEN t1.SP_VALUE_LOW IS NULL THEN 1 ELSE 0 END DESC, t1.SP_VALUE_LOW DESC limit ?")); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW DESC limit ?")); + + generatedSql = buildSqlWithDateSort(true, OrderObject.NullOrder.LAST); +// assertTrue(generatedSql.getSql().endsWith("ORDER BY CASE WHEN t1.SP_VALUE_LOW IS NULL THEN 1 ELSE 0 END ASC, t1.SP_VALUE_LOW ASC limit ?")); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW ASC limit ?")); + + generatedSql = buildSqlWithDateSort(false, OrderObject.NullOrder.LAST); + assertTrue(generatedSql.getSql().endsWith("ORDER BY t1.SP_VALUE_LOW DESC limit ?")); + + } +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderTest.java index e6ba8aabf56..8ae2e33d00d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderTest.java @@ -11,7 +11,6 @@ import com.google.common.collect.Lists; import org.hibernate.dialect.DerbyTenSevenDialect; import org.hibernate.dialect.MariaDB103Dialect; import org.hibernate.dialect.MySQL8Dialect; -import org.hibernate.dialect.MySQLDialect; import org.hibernate.dialect.Oracle12cDialect; import org.hibernate.dialect.PostgreSQL95Dialect; import org.hibernate.dialect.SQLServer2005Dialect; @@ -83,22 +82,25 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new SQLServer2005Dialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range generated = builder.generate(null, null); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L)); // Max only generated = builder.generate(null, 10); - assertEquals("SELECT TOP(?) t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST", generated.getSql()); +// assertEquals("SELECT TOP(?) t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC", generated.getSql()); + assertEquals("SELECT TOP(?) t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains(10, "Patient", 500L, 501L)); // Range generated = builder.generate(10, 5); - assertEquals("WITH query AS (SELECT inner_query.*, ROW_NUMBER() OVER (ORDER BY CURRENT_TIMESTAMP) as __hibernate_row_nr__ FROM ( SELECT TOP(?) t0.RES_ID as page0_ FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST ) inner_query ) SELECT page0_ FROM query WHERE __hibernate_row_nr__ >= ? AND __hibernate_row_nr__ < ?", generated.getSql()); +// assertEquals("WITH query AS (SELECT inner_query.*, ROW_NUMBER() OVER (ORDER BY CURRENT_TIMESTAMP) as __hibernate_row_nr__ FROM ( SELECT TOP(?) t0.RES_ID as page0_ FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC ) inner_query ) SELECT page0_ FROM query WHERE __hibernate_row_nr__ >= ? AND __hibernate_row_nr__ < ?", generated.getSql()); + assertEquals("WITH query AS (SELECT inner_query.*, ROW_NUMBER() OVER (ORDER BY CURRENT_TIMESTAMP) as __hibernate_row_nr__ FROM ( SELECT TOP(?) t0.RES_ID as page0_ FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC ) inner_query ) SELECT page0_ FROM query WHERE __hibernate_row_nr__ >= ? AND __hibernate_row_nr__ < ?", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains(5, "Patient", 500L, 501L, 11, 16)); } @@ -137,22 +139,25 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new SQLServer2012Dialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range generated = builder.generate(null, null); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L)); // Max only generated = builder.generate(null, 10); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST offset 0 rows fetch next ? rows only", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC offset 0 rows fetch next ? rows only", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC offset 0 rows fetch next ? rows only", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L, 10)); // Range generated = builder.generate(10, 5); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST offset ? rows fetch next ? rows only", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC offset ? rows fetch next ? rows only", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC offset ? rows fetch next ? rows only", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L, 10, 5)); } @@ -190,7 +195,7 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new PostgreSQL95Dialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range @@ -243,7 +248,7 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new Oracle12cDialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range @@ -297,22 +302,25 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new MySQL8Dialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range generated = builder.generate(null, null); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L)); // Max only generated = builder.generate(null, 10); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST limit ?", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC limit ?", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC limit ?", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L, 10)); // Range generated = builder.generate(10, 5); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST limit ?, ?", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC limit ?, ?", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC limit ?, ?", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L, 10, 5)); } @@ -351,22 +359,25 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new MariaDB103Dialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range generated = builder.generate(null, null); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L)); // Max only generated = builder.generate(null, 10); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST limit ?", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC limit ?", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC limit ?", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L, 10)); // Range generated = builder.generate(10, 5); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC NULLS LAST limit ?, ?", generated.getSql()); +// assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY CASE WHEN t0.RES_UPDATED IS NULL THEN 1 ELSE 0 END ASC, t0.RES_UPDATED ASC limit ?, ?", generated.getSql()); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND (t0.RES_ID IN (?,?) )) ORDER BY t0.RES_UPDATED ASC limit ?, ?", generated.getSql()); assertThat(generated.getBindVariables().toString(), generated.getBindVariables(), contains("Patient", 500L, 501L, 10, 5)); } @@ -405,7 +416,7 @@ public class SearchQueryBuilderTest { dialectProvider.setDialectForUnitTest(new DerbyTenSevenDialect()); SearchQueryBuilder builder = new SearchQueryBuilder(myFhirContext, myModelConfig, myPartitionSettings, myRequestPartitionId, "Patient", mySqlBuilderFactory, dialectProvider, false); builder.addResourceIdsPredicate(Lists.newArrayList(500L, 501L)); - builder.addSort(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); + builder.addSortDate(builder.getOrCreateResourceTablePredicateBuilder().getColumnLastUpdated(), true); GeneratedSql generated; // No range