From 75f7defc748ca5a35b9c3a7c82c4f65e43aec4ae Mon Sep 17 00:00:00 2001 From: Michael Buckley Date: Fri, 27 Sep 2024 17:02:31 -0400 Subject: [PATCH] Give up on nullable join :not joins require sub-selects and the nullability can't support an in(select ) clause. Bring in flag, and set based on partition settings --- .../fhir/jpa/search/builder/QueryStack.java | 54 ++++++++++------ .../builder/sql/SearchQueryBuilder.java | 63 +++++++++---------- .../builder/sql/SearchQueryExecutor.java | 21 +++++-- .../jpa/model/config/PartitionSettings.java | 19 ++++++ .../r4/FhirResourceDaoR4SearchNoFtTest.java | 2 +- .../r4/FhirResourceDaoR4SearchSqlTest.java | 21 ++++++- .../jpa/dao/r4/PartitioningSqlR4Test.java | 10 +-- 7 files changed, 124 insertions(+), 66 deletions(-) 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 de51f138575..5f0f55e1b3f 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 @@ -55,6 +55,7 @@ import ca.uhn.fhir.jpa.search.builder.predicate.StringPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.TagPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.TokenPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.UriPredicateBuilder; +import ca.uhn.fhir.jpa.search.builder.sql.ColumnTupleObject; import ca.uhn.fhir.jpa.search.builder.sql.PredicateBuilderFactory; import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; @@ -359,7 +360,8 @@ public class QueryStack { case STRING: StringPredicateBuilder stringPredicateBuilder = mySqlBuilder.createStringPredicateBuilder(); addSortCustomJoin( - resourceLinkPredicateBuilder.getColumnTargetResourceId(), + // fixme switch? + resourceLinkPredicateBuilder.getJoinColumnsForTarget(), stringPredicateBuilder, stringPredicateBuilder.createHashIdentityPredicate(targetType, theChain)); @@ -370,7 +372,7 @@ public class QueryStack { case TOKEN: TokenPredicateBuilder tokenPredicateBuilder = mySqlBuilder.createTokenPredicateBuilder(); addSortCustomJoin( - resourceLinkPredicateBuilder.getColumnTargetResourceId(), + resourceLinkPredicateBuilder.getJoinColumnsForTarget(), tokenPredicateBuilder, tokenPredicateBuilder.createHashIdentityPredicate(targetType, theChain)); @@ -381,7 +383,7 @@ public class QueryStack { case DATE: DatePredicateBuilder datePredicateBuilder = mySqlBuilder.createDatePredicateBuilder(); addSortCustomJoin( - resourceLinkPredicateBuilder.getColumnTargetResourceId(), + resourceLinkPredicateBuilder.getJoinColumnsForTarget(), datePredicateBuilder, datePredicateBuilder.createHashIdentityPredicate(targetType, theChain)); @@ -476,15 +478,16 @@ public class QueryStack { BaseJoiningPredicateBuilder theToJoiningPredicateBuilder, Condition theCondition) { addSortCustomJoin( - theFromJoiningPredicateBuilder.getResourceIdColumn(), theToJoiningPredicateBuilder, theCondition); + theFromJoiningPredicateBuilder.getJoinColumns(), theToJoiningPredicateBuilder, theCondition); } private void addSortCustomJoin( - DbColumn theFromDbColumn, + DbColumn theFromDbColumn[], BaseJoiningPredicateBuilder theToJoiningPredicateBuilder, Condition theCondition) { + ComboCondition onCondition = - mySqlBuilder.createOnCondition(theFromDbColumn, theToJoiningPredicateBuilder.getResourceIdColumn()); + mySqlBuilder.createOnCondition(theFromDbColumn, theToJoiningPredicateBuilder.getJoinColumns()); if (theCondition != null) { onCondition.addCondition(theCondition); @@ -492,7 +495,7 @@ public class QueryStack { mySqlBuilder.addCustomJoin( SelectQuery.JoinType.LEFT_OUTER, - theFromDbColumn.getTable(), + theFromDbColumn[0].getTable(), theToJoiningPredicateBuilder.getTable(), onCondition); } @@ -1476,12 +1479,12 @@ public class QueryStack { public void addGrouping() { BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); - mySqlBuilder.getSelect().addGroupings(firstPredicateBuilder.getResourceIdColumn()); + mySqlBuilder.getSelect().addGroupings(firstPredicateBuilder.getJoinColumns()); } public void addOrdering() { BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); - mySqlBuilder.getSelect().addOrderings(firstPredicateBuilder.getResourceIdColumn()); + mySqlBuilder.getSelect().addOrderings(firstPredicateBuilder.getJoinColumns()); } public Condition createPredicateReferenceForEmbeddedChainedSearchResource( @@ -1529,7 +1532,7 @@ public class QueryStack { for (LeafNodeDefinition leafNodeDefinition : referenceLinks.get(nextReferenceLink)) { SearchQueryBuilder builder; if (wantChainedAndNormal) { - builder = mySqlBuilder.newChildSqlBuilder(); + builder = mySqlBuilder.newChildSqlBuilder(mySqlBuilder.isIncludePartitionIdInJoins()); } else { builder = mySqlBuilder; } @@ -1574,8 +1577,11 @@ public class QueryStack { if (wantChainedAndNormal) { if (theSourceJoinColumn == null) { - retVal = new InCondition( - mySqlBuilder.getOrCreateFirstPredicateBuilder(false).getResourceIdColumn(), union); + BaseJoiningPredicateBuilder root = mySqlBuilder.getOrCreateFirstPredicateBuilder(false); + DbColumn[] joinColumns = root.getJoinColumns(); + // fixme check for length 1 and skip tuple wrap. + Object joinColumnObject = ColumnTupleObject.from(joinColumns); + retVal = new InCondition(joinColumnObject, union); } else { // -- for the resource link, need join with target_resource_id retVal = new InCondition(theSourceJoinColumn, union); @@ -2050,7 +2056,8 @@ public class QueryStack { BaseJoiningPredicateBuilder join; if (paramInverted) { - SearchQueryBuilder sqlBuilder = mySqlBuilder.newChildSqlBuilder(); + boolean selectPartitionId = myPartitionSettings.isPartitionIdsInPrimaryKeys(); + SearchQueryBuilder sqlBuilder = mySqlBuilder.newChildSqlBuilder(selectPartitionId); TagPredicateBuilder tagSelector = sqlBuilder.addTagPredicateBuilder(null); sqlBuilder.addPredicate( tagSelector.createPredicateTag(tagType, tokens, theParamName, theRequestPartitionId)); @@ -2058,7 +2065,14 @@ public class QueryStack { join = mySqlBuilder.getOrCreateFirstPredicateBuilder(); Expression subSelect = new Subquery(sql); - tagPredicate = new InCondition(join.getResourceIdColumn(), subSelect).setNegate(true); + + Object left; + if (selectPartitionId) { + left = new ColumnTupleObject(join.getJoinColumns()); + } else { + left = join.getResourceIdColumn(); + } + tagPredicate = new InCondition(left, subSelect).setNegate(true); } else { // Tag table can't be a query root because it will include deleted resources, and can't select by @@ -2221,7 +2235,8 @@ public class QueryStack { BaseJoiningPredicateBuilder join; if (paramInverted) { - SearchQueryBuilder sqlBuilder = theSqlBuilder.newChildSqlBuilder(); + boolean selectPartitionId = myPartitionSettings.isPartitionIdsInPrimaryKeys(); + SearchQueryBuilder sqlBuilder = theSqlBuilder.newChildSqlBuilder(selectPartitionId); TokenPredicateBuilder tokenSelector = sqlBuilder.addTokenPredicateBuilder(null); sqlBuilder.addPredicate(tokenSelector.createPredicateToken( tokens, theResourceName, theSpnamePrefix, theSearchParam, theRequestPartitionId)); @@ -2230,13 +2245,16 @@ public class QueryStack { join = theSqlBuilder.getOrCreateFirstPredicateBuilder(); + DbColumn[] leftColumns; if (theSourceJoinColumn == null) { - predicate = new InCondition(join.getResourceIdColumn(), subSelect).setNegate(true); + leftColumns = join.getJoinColumns(); } else { - // -- for the resource link, need join with target_resource_id - predicate = new InCondition(theSourceJoinColumn, subSelect).setNegate(true); + leftColumns = theSourceJoinColumn; } + Object left = new ColumnTupleObject(leftColumns); + predicate = new InCondition(left, subSelect).setNegate(true); + } else { Boolean isMissing = theList.get(0).getMissing(); if (isMissing != null) { 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 c44f699fc23..0c488699f66 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 @@ -55,8 +55,9 @@ import com.healthmarketscience.sqlbuilder.FunctionCall; import com.healthmarketscience.sqlbuilder.InCondition; import com.healthmarketscience.sqlbuilder.OrderObject; import com.healthmarketscience.sqlbuilder.SelectQuery; -import com.healthmarketscience.sqlbuilder.UnaryCondition; +import com.healthmarketscience.sqlbuilder.dbspec.Join; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbColumn; +import com.healthmarketscience.sqlbuilder.dbspec.basic.DbJoin; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSchema; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSpec; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbTable; @@ -74,7 +75,6 @@ import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Locale; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -102,6 +102,7 @@ public class SearchQueryBuilder { private final SqlObjectFactory mySqlBuilderFactory; private final boolean myCountQuery; private final Dialect myDialect; + private final boolean mySelectPartitionId; private boolean myMatchNothing; private ResourceTablePredicateBuilder myResourceTableRoot; private boolean myHaveAtLeastOnePredicate; @@ -135,7 +136,8 @@ public class SearchQueryBuilder { UUID.randomUUID() + "-", theDialectProvider.getDialect(), theCountQuery, - new ArrayList<>()); + new ArrayList<>(), + thePartitionSettings.isPartitionIdsInPrimaryKeys()); } /** @@ -151,7 +153,8 @@ public class SearchQueryBuilder { String theBindVariableSubstitutionBase, Dialect theDialect, boolean theCountQuery, - ArrayList theBindVariableValues) { + ArrayList theBindVariableValues, + boolean theSelectPartitionId) { myFhirContext = theFhirContext; myStorageSettings = theStorageSettings; myPartitionSettings = thePartitionSettings; @@ -173,6 +176,7 @@ public class SearchQueryBuilder { myBindVariableSubstitutionBase = theBindVariableSubstitutionBase; myBindVariableValues = theBindVariableValues; + mySelectPartitionId = theSelectPartitionId; } public FhirContext getFhirContext() { @@ -359,13 +363,15 @@ public class SearchQueryBuilder { mySelect.addCustomJoin(theJoinType, theFromTable, theToTable, theCondition); } - public ComboCondition createOnCondition(DbColumn theSourceColumn, DbColumn theTargetColumn) { + public ComboCondition createOnCondition(DbColumn[] theSourceColumn, DbColumn[] theTargetColumn) { ComboCondition onCondition = ComboCondition.and(); - onCondition.addCondition(BinaryCondition.equalTo(theSourceColumn, theTargetColumn)); - + for (int i = 0; i < theSourceColumn.length; i+=1) { + onCondition.addCondition(BinaryCondition.equalTo(theSourceColumn[0], theTargetColumn[0])); + } return onCondition; } + /** * Add and return a predicate builder (or a root query if no root query exists yet) for selecting on a :missing search parameter */ @@ -433,10 +439,17 @@ public class SearchQueryBuilder { if (myCountQuery) { mySelect.addCustomColumns( FunctionCall.count().setIsDistinct(true).addColumnParams(root.getResourceIdColumn())); + } else { + if (mySelectPartitionId) { + mySelectedResourceIdColumn = root.getResourceIdColumn(); + mySelectedPartitionIdColumn = root.getPartitionIdColumn(); + // fixme reverse? + mySelect.addColumns(mySelectedPartitionIdColumn, mySelectedResourceIdColumn); } else { mySelectedResourceIdColumn = root.getResourceIdColumn(); mySelect.addColumns(mySelectedResourceIdColumn); } + } mySelect.addFromTable(root.getTable()); myFirstPredicateBuilder = root; @@ -460,9 +473,13 @@ public class SearchQueryBuilder { return toJoinColumns(partitionIdColumn, resourceIdColumn); } + /** + * Remove or keep partition_id columns depending on settings. + */ @Nonnull public DbColumn[] toJoinColumns(DbColumn partitionIdColumn, DbColumn resourceIdColumn) { if (isIncludePartitionIdInJoins()) { + // fixme can we reverse these? return new DbColumn[] {partitionIdColumn, resourceIdColumn}; } else { return new DbColumn[] {resourceIdColumn}; @@ -470,7 +487,7 @@ public class SearchQueryBuilder { } public boolean isIncludePartitionIdInJoins() { - return true; + return mySelectPartitionId && myPartitionSettings.isPartitionIdsInPrimaryKeys(); } public void addJoin(DbTable theFromTable, DbTable theToTable, DbColumn[] theFromColumn, DbColumn[] theToColumn) { @@ -484,29 +501,8 @@ public class SearchQueryBuilder { DbColumn[] theToColumn, SelectQuery.JoinType theJoinType) { assert theFromColumn.length == theToColumn.length; - assert theFromColumn.length > 0; - // create custom join condition, allowing nullable columns. - var onCondition = ComboCondition.and(); - for (int i = 0; i < theFromColumn.length; ++i) { - boolean isNullable = - theFromColumn[i].getColumnNameSQL().toLowerCase(Locale.ROOT).contains("partition_id"); - onCondition.addCondition(buildJoinColumnCondition(isNullable, theFromColumn[i], theToColumn[i])); - } - mySelect.addCustomJoin(theJoinType, theFromTable, theToTable, onCondition); - } - - private static @Nonnull Condition buildJoinColumnCondition( - boolean theColumnNullability, DbColumn theFromColumn, DbColumn theToColumn) { - var normalEqualCondition = BinaryCondition.equalTo(theFromColumn, theToColumn); - if (!theColumnNullability) { - return normalEqualCondition; - } else { - // the column can be null - // we must combine raw = with IS NULL checks. - var orBothNullCondition = - ComboCondition.and(UnaryCondition.isNull(theFromColumn), UnaryCondition.isNull(theToColumn)); - return ComboCondition.or(normalEqualCondition, orBothNullCondition); - } + Join join = new DbJoin(mySpec, theFromTable, theToTable, theFromColumn, theToColumn); + mySelect.addJoins(theJoinType, join); } /** @@ -842,7 +838,7 @@ public class SearchQueryBuilder { } } - public SearchQueryBuilder newChildSqlBuilder() { + public SearchQueryBuilder newChildSqlBuilder(boolean theSelectPartitionId) { return new SearchQueryBuilder( myFhirContext, myStorageSettings, @@ -853,7 +849,8 @@ public class SearchQueryBuilder { myBindVariableSubstitutionBase, myDialect, false, - myBindVariableValues); + myBindVariableValues, + theSelectPartitionId); } public SelectQuery getSelect() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java index 3b7b9d0d0fc..3f3ff9a8828 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.search.builder.sql; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; +import ca.uhn.fhir.jpa.model.dao.JpaPid; import ca.uhn.fhir.jpa.search.builder.ISearchQueryExecutor; import ca.uhn.fhir.jpa.util.ScrollableResultsIterator; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; @@ -107,6 +108,7 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { * is managed by Spring has been started before this method is called. */ HapiTransactionService.requireTransaction(); + ourLog.trace("About to execute SQL: {}. Parameters: {}", sql, Arrays.toString(args)); Query nativeQuery = myEntityManager.createNativeQuery(sql); org.hibernate.query.Query hibernateQuery = (org.hibernate.query.Query) nativeQuery; @@ -114,8 +116,6 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { hibernateQuery.setParameter(i, args[i - 1]); } - ourLog.trace("About to execute SQL: {}. Parameters: {}", sql, Arrays.toString(args)); - /* * These settings help to ensure that we use a search cursor * as opposed to loading all search results into memory @@ -146,13 +146,22 @@ public class SearchQueryExecutor implements ISearchQueryExecutor { myNext = NO_MORE; } else { Object nextRow = Objects.requireNonNull(myResultSet.next()); - Number next; + // We should typically get two columns back, the first is the partition ID and the second + // is the resource ID. But if we're doing a count query, we'll get a single column in an array + // or maybe even just a single non array value depending on how the platform handles it. if (nextRow instanceof Number) { - next = (Number) nextRow; + myNext = ((Number) nextRow).longValue(); } else { - next = (Number) ((Object[]) nextRow)[0]; + Object[] nextRowAsArray = (Object[]) nextRow; + if (nextRowAsArray.length == 1) { + myNext = (Long) nextRowAsArray[0]; + } else { + // fixme reverse + Integer nextPartitionId = (Integer) nextRowAsArray[0]; + Long nextResourceId = (Long) nextRowAsArray[1]; + myNext = nextResourceId; + } } - myNext = next.longValue(); } } catch (Exception e) { diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java index 379f8098f23..374a18ddd94 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java @@ -33,8 +33,27 @@ public class PartitionSettings { private Integer myDefaultPartitionId; private boolean myAlwaysOpenNewTransactionForDifferentPartition; private boolean myConditionalCreateDuplicateIdentifiersEnabled = false; + private boolean myPartitionIdsInPrimaryKeys = false; public PartitionSettings() {} + + /** + * Is the table partition column (usually PARTITION_ID) + * participating and primary and foreign keys. + * Affects sql joins, sql in() expressions, etc. + */ + public boolean isPartitionIdsInPrimaryKeys() { + return myPartitionIdsInPrimaryKeys; + } + + /** + * Inform the query engine if the primary and foreign keys + * of the partitioned tables include the partition column. + */ + public void setPartitionIdsInPrimaryKeys(boolean thePartitionIdsInPrimaryKeys) { + myPartitionIdsInPrimaryKeys = thePartitionIdsInPrimaryKeys; + } + /** * Should we always open a new database transaction if the partition context changes * diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 0ff5dc2c417..3cb42abf4c5 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -4557,7 +4557,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { assertThat(patients).as(patients.toString()).containsExactly(obsId1); String searchQuery = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search query:\n{}", searchQuery); - assertThat(countMatches(searchQuery.toLowerCase(), "partition")).as(searchQuery).isEqualTo(0 /* criteria */ + 8 /* joins */); + assertThat(countMatches(searchQuery.toLowerCase(), "partition")).as(searchQuery).isEqualTo(0); assertThat(countMatches(searchQuery.toLowerCase(), "join")).as(searchQuery).isEqualTo(2); assertThat(countMatches(searchQuery.toLowerCase(), "hash_identity")).as(searchQuery).isEqualTo(2); // - query is changed 'or' is removed diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchSqlTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchSqlTest.java index 99514093bb3..3cc8e9c8294 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchSqlTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchSqlTest.java @@ -1,10 +1,10 @@ package ca.uhn.fhir.jpa.dao.r4; -import static org.junit.jupiter.api.Assertions.assertEquals; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; @@ -17,9 +17,11 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; public class FhirResourceDaoR4SearchSqlTest extends BaseJpaR4Test { @@ -52,6 +54,19 @@ public class FhirResourceDaoR4SearchSqlTest extends BaseJpaR4Test { } + @Test + public void testSortJoinIncludesPartitionId() { + + myCaptureQueriesListener.clear(); + SearchParameterMap map = SearchParameterMap.newSynchronous(Patient.SP_ACTIVE, new TokenParam("true")); + map.setSort(new SortSpec(Patient.SP_NAME)); + myPatientDao.search(map); + assertEquals(1, myCaptureQueriesListener.countSelectQueries()); + String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(false, false); + // fixme log bug about duplicate join of string in sort when also in query + assertEquals("SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_SPIDX_TOKEN t0 ON (t1.RES_ID = t0.RES_ID) LEFT OUTER JOIN HFJ_SPIDX_STRING t2 ON ((t1.RES_ID = t2.RES_ID) AND (t2.HASH_IDENTITY = ?)) WHERE (t0.HASH_VALUE = ?) ORDER BY t2.SP_VALUE_NORMALIZED ASC NULLS LAST", sql); + + } /** * Two regular search params - Should use HFJ_RESOURCE as root */ @@ -65,7 +80,7 @@ public class FhirResourceDaoR4SearchSqlTest extends BaseJpaR4Test { myPatientDao.search(map); assertEquals(1, myCaptureQueriesListener.countSelectQueries()); String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(false, false); - assertEquals("SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_SPIDX_STRING t0 ON (((t1.PARTITION_ID = t0.PARTITION_ID) OR ((t1.PARTITION_ID IS NULL) AND (t0.PARTITION_ID IS NULL))) AND (t1.RES_ID = t0.RES_ID)) INNER JOIN HFJ_SPIDX_TOKEN t2 ON (((t1.PARTITION_ID = t2.PARTITION_ID) OR ((t1.PARTITION_ID IS NULL) AND (t2.PARTITION_ID IS NULL))) AND (t1.RES_ID = t2.RES_ID)) WHERE (((t0.HASH_NORM_PREFIX = ?) AND (t0.SP_VALUE_NORMALIZED LIKE ?)) AND (t2.HASH_SYS_AND_VALUE = ?))", sql); + assertEquals("SELECT t1.RES_ID FROM HFJ_RESOURCE t1 INNER JOIN HFJ_SPIDX_STRING t0 ON (t1.RES_ID = t0.RES_ID) INNER JOIN HFJ_SPIDX_TOKEN t2 ON (t1.RES_ID = t2.RES_ID) WHERE (((t0.HASH_NORM_PREFIX = ?) AND (t0.SP_VALUE_NORMALIZED LIKE ?)) AND (t2.HASH_SYS_AND_VALUE = ?))", sql); } @Test @@ -86,7 +101,7 @@ public class FhirResourceDaoR4SearchSqlTest extends BaseJpaR4Test { assertEquals(3, myCaptureQueriesListener.countSelectQueries()); // Query 1 - Find resources: Make sure we search for tag type+system+code always String sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(false, false); - assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 INNER JOIN HFJ_RES_TAG t1 ON (((t0.PARTITION_ID = t1.PARTITION_ID) OR ((t0.PARTITION_ID IS NULL) AND (t1.PARTITION_ID IS NULL))) AND (t0.RES_ID = t1.RES_ID)) INNER JOIN HFJ_TAG_DEF t2 ON (t1.TAG_ID = t2.TAG_ID) WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND ((t2.TAG_TYPE = ?) AND (t2.TAG_SYSTEM = ?) AND (t2.TAG_CODE = ?)))", sql); + assertEquals("SELECT t0.RES_ID FROM HFJ_RESOURCE t0 INNER JOIN HFJ_RES_TAG t1 ON (t0.RES_ID = t1.RES_ID) INNER JOIN HFJ_TAG_DEF t2 ON (t1.TAG_ID = t2.TAG_ID) WHERE (((t0.RES_TYPE = ?) AND (t0.RES_DELETED_AT IS NULL)) AND ((t2.TAG_TYPE = ?) AND (t2.TAG_SYSTEM = ?) AND (t2.TAG_CODE = ?)))", sql); // Query 2 - Load resourece contents sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(1).getSql(false, false); assertThat(sql).contains("where rsv1_0.RES_ID in (?)"); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java index 433ca014bb8..9eddb6767e8 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java @@ -1327,7 +1327,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { // Only the read columns should be used, no criteria use partition assertThat(searchSql).as(searchSql).contains("PARTITION_ID = '1'"); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2 /* criteria */ + 4 /* joins */); // If this switches to 1 that would be fine + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); // If this switches to 1 that would be fine } // Read in null Partition @@ -1443,7 +1443,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(1).getSql(true, false).toUpperCase(); ourLog.info("Search SQL:\n{}", searchSql); assertThat(searchSql).as(searchSql).contains("PARTITION_ID = '1'"); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2 /* criteria */ + 4 /* joins */); // If this switches to 1 that would be fine + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); // If this switches to 1 that would be fine } // Read in null Partition @@ -2383,7 +2383,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(0 /* criteria */ + 4 /* joins */); + assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(0); assertThat(StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")).as(searchSql).isEqualTo(1); assertThat(StringUtils.countMatches(searchSql, ".HASH_SYS_AND_VALUE =")).as(searchSql).isEqualTo(1); @@ -2457,7 +2457,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { String searchSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, true); ourLog.info("Search SQL:\n{}", searchSql); - assertEquals(0 /* criteria */ + 4 /* joins */, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(0, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); } @@ -2483,7 +2483,7 @@ public class PartitioningSqlR4Test extends BasePartitioningR4Test { ourLog.info("Search SQL:\n{}", searchSql); assertEquals(2, StringUtils.countMatches(searchSql, "JOIN")); - assertEquals(1 /* criteria */ + 4 /* joins */, StringUtils.countMatches(searchSql, "PARTITION_ID")); + assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID")); assertEquals(1, StringUtils.countMatches(searchSql, "TAG_SYSTEM = 'http://system'")); }