cleanup

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

@@ -1574,7 +1574,6 @@ public class QueryStack {
 		if (wantChainedAndNormal) {
 
 			if (theSourceJoinColumn == null) {
-				// fixme mb HERE!!!
 				retVal = new InCondition(
 						mySqlBuilder.getOrCreateFirstPredicateBuilder(false).getResourceIdColumn(), union);
 			} else {

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

@@ -26,7 +26,6 @@ import ca.uhn.fhir.jpa.model.dao.JpaPid;
 import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder;
 import ca.uhn.fhir.jpa.util.QueryParameterUtils;
 import ca.uhn.fhir.model.api.IQueryParameterType;
-import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.rest.param.TokenParamModifier;
 import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
@@ -136,7 +135,6 @@ public class ResourceIdPredicateBuilder extends BasePredicateBuilder {
 				}
 			} else {
 				DbColumn resIdColumn = getResourceIdColumn(theSourceJoinColumn);
-				// FIXME: find a test that lands here and make sure we have a Keep test doing the same
 				return QueryParameterUtils.toEqualToOrInPredicate(
 						resIdColumn,
 						generatePlaceholders(resourceIds),

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

@@ -182,7 +182,6 @@ public class ResourceLinkPredicateBuilder extends BaseJoiningPredicateBuilder im
 	 * one of these, use {@link #getJoinColumnsForSource()} or {@link #getJoinColumnsForTarget()}.
 	 */
 	@Override
-	// fixme dig into this
 	public DbColumn[] getJoinColumns() {
 		return super.getJoinColumns();
 	}

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

@@ -1,64 +0,0 @@
-package ca.uhn.fhir.jpa.search.builder.sql;
-
-import ca.uhn.fhir.jpa.model.dao.JpaPid;
-import com.healthmarketscience.common.util.AppendableExt;
-import com.healthmarketscience.sqlbuilder.Expression;
-import com.healthmarketscience.sqlbuilder.ValidationContext;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-
-/**
- * Outputs an SQL tuple for a collection of JpaPids, consisting of
- * ((resId,partitionId),(resId,partitionId),(resId,partitionId),...)
- */
-public class JpaPidValueTupleObject extends Expression {
-
-	private final Collection<String> myValues;
-
-	public JpaPidValueTupleObject(Collection<String> theValues) {
-		myValues = theValues;
-	}
-
-	@Override
-	protected void collectSchemaObjects(ValidationContext vContext) {
-		// nothing
-	}
-
-	@Override
-	public void appendTo(AppendableExt app) throws IOException {
-		app.append('(');
-
-		String value;
-		for (Iterator<String> iter = myValues.iterator(); iter.hasNext(); ) {
-			if (hasParens()) {
-				// fixme do we want the quotes?  I think these are numbers.
-				app.append("('");
-			}
-			value = iter.next();
-			app.append(value);
-			app.append("','");
-			value = iter.next();
-			app.append(value);
-			app.append("')");
-			if (iter.hasNext()) {
-				app.append(',');
-			}
-		}
-		if (hasParens()) {
-			app.append(')');
-		}
-	}
-
-	public static JpaPidValueTupleObject from(SearchQueryBuilder theSearchQueryBuilder, JpaPid[] thePids) {
-		List<String> placeholders = new ArrayList<>(thePids.length * 2);
-		for (JpaPid next : thePids) {
-			placeholders.add(theSearchQueryBuilder.generatePlaceholder(next.getPartitionId()));
-			placeholders.add(theSearchQueryBuilder.generatePlaceholder(next.getId()));
-		}
-		return new JpaPidValueTupleObject(placeholders);
-	}
-}

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

@@ -55,9 +55,8 @@ import com.healthmarketscience.sqlbuilder.FunctionCall;
 import com.healthmarketscience.sqlbuilder.InCondition;
 import com.healthmarketscience.sqlbuilder.OrderObject;
 import com.healthmarketscience.sqlbuilder.SelectQuery;
-import com.healthmarketscience.sqlbuilder.dbspec.Join;
+import com.healthmarketscience.sqlbuilder.UnaryCondition;
 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;
@@ -75,6 +74,7 @@ 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;
@@ -484,8 +484,29 @@ public class SearchQueryBuilder {
 			DbColumn[] theToColumn,
 			SelectQuery.JoinType theJoinType) {
 		assert theFromColumn.length == theToColumn.length;
-		Join join = new DbJoin(mySpec, theFromTable, theToTable, theFromColumn, theToColumn);
+		assert theFromColumn.length > 0;
-		mySelect.addJoins(theJoinType, join);
+		// 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);
+		}
 	}
 
 	/**
@@ -678,7 +699,7 @@ public class SearchQueryBuilder {
 	 * for its generated SQL. So we work around this by replacing our contents with a string in the SQL consisting
 	 * of <code>[random UUID]-[value index]</code> and then
 	 */
-	public String  generatePlaceholder(Object theValue) {
+	public String generatePlaceholder(Object theValue) {
 		String placeholder = myBindVariableSubstitutionBase + myBindVariableValues.size();
 		myBindVariableValues.add(theValue);
 		return placeholder;

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

@@ -65,9 +65,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.RES_ID = t0.RES_ID AND ((t1.PARTITION_ID = t0.PARTITION_ID) OR (t1.PARTITION_ID is null AND t0.PARTITION_ID is null))) INNER JOIN HFJ_SPIDX_TOKEN t2 ON (t1.RES_ID = t2.RES_ID AND ((t1.PARTITION_ID = t2.PARTITION_ID) OR (t1.PARTITION_ID is null AND t2.PARTITION_ID is null)) ) 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.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);
-
-
 	}
 
 	@Test
@@ -88,7 +86,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.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.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);
 		// Query 2 - Load resourece contents
 		sql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(1).getSql(false, false);
 		assertThat(sql).contains("where rsv1_0.RES_ID in (?)");