Improve ID filtering (#5215)

* Improve ID filtering

* Formatting

* Add license header

* Address review comments

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5215-improve-hfql-id-filtering.yaml

@@ -0,0 +1,5 @@
+---
+type: perf
+issue: 5215
+title: "When executing an HFQL search with a FHIRPath filter on the `id` element, this will now
+  be automatically converted into an `_id` search parameter match for better performance."

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

@@ -1,3 +1,22 @@
+/*-
+ * #%L
+ * HAPI FHIR JPA Server
+ * %%
+ * Copyright (C) 2014 - 2023 Smile CDR, Inc.
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
 package ca.uhn.fhir.jpa.search.builder.models;
 
 import ca.uhn.fhir.jpa.search.builder.ISearchQueryExecutor;

hapi-fhir-jpaserver-hfql/src/main/java/ca/uhn/fhir/jpa/fql/executor/HfqlExecutor.java

@@ -42,11 +42,13 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.param.DateOrListParam;
 import ca.uhn.fhir.rest.param.DateParam;
+import ca.uhn.fhir.rest.param.ParameterUtil;
 import ca.uhn.fhir.rest.param.QualifierDetails;
 import ca.uhn.fhir.rest.param.TokenOrListParam;
 import ca.uhn.fhir.rest.server.IPagingProvider;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
+import ca.uhn.fhir.rest.server.util.ResourceSearchParams;
 import ca.uhn.fhir.util.UrlUtil;
 import com.google.common.collect.Lists;
 import org.apache.commons.collections4.ListUtils;
@@ -142,6 +144,8 @@ public class HfqlExecutor implements IHfqlExecutor {
 
 		massageSelectColumnNames(statement);
 		populateSelectColumnDataTypes(statement);
+		validateWhereClauses(statement);
+		massageWhereClauses(statement);
 
 		SearchParameterMap map = new SearchParameterMap();
 		addHfqlWhereClausesToSearchParameterMap(statement, map);
@@ -179,6 +183,39 @@ public class HfqlExecutor implements IHfqlExecutor {
 		return executionResult;
 	}
 
+	private void validateWhereClauses(HfqlStatement theStatement) {
+		for (HfqlStatement.WhereClause next : theStatement.getWhereClauses()) {
+			if (isDataValueWhereClause(next)) {
+				if (next.getLeft().matches("^[a-zA-Z]+$")) {
+					RuntimeResourceDefinition resDef =
+							myFhirContext.getResourceDefinition(theStatement.getFromResourceName());
+					if (resDef.getChildByName(next.getLeft()) == null) {
+						throw new InvalidRequestException(
+								Msg.code(2429) + "Resource type " + theStatement.getFromResourceName()
+										+ " does not have a root element named '" + next.getLeft() + "'");
+					}
+				}
+			}
+		}
+	}
+
+	private void massageWhereClauses(HfqlStatement theStatement) {
+		ResourceSearchParams activeSearchParams =
+				mySearchParamRegistry.getActiveSearchParams(theStatement.getFromResourceName());
+
+		for (HfqlStatement.WhereClause nextWhereClause : theStatement.getWhereClauses()) {
+			if (isDataValueWhereClause(nextWhereClause)) {
+				if ("id".equals(nextWhereClause.getLeft())) {
+					nextWhereClause.setOperator(HfqlStatement.WhereClauseOperatorEnum.SEARCH_MATCH);
+					String joinedParamValues = nextWhereClause.getRightAsStrings().stream()
+							.map(ParameterUtil::escape)
+							.collect(Collectors.joining(","));
+					nextWhereClause.setRight("_id", joinedParamValues);
+				}
+			}
+		}
+	}
+
 	private void addHfqlWhereClausesToSearchParameterMap(HfqlStatement statement, SearchParameterMap map) {
 		List<HfqlStatement.WhereClause> searchClauses = statement.getWhereClauses();
 		for (HfqlStatement.WhereClause nextSearchClause : searchClauses) {
@@ -740,6 +777,17 @@ public class HfqlExecutor implements IHfqlExecutor {
 		return new StaticHfqlExecutionResult(null, columns, dataTypes, rows);
 	}
 
+	/**
+	 * Returns {@literal true} if a where clause has an operator of
+	 * {@link ca.uhn.fhir.jpa.fql.parser.HfqlStatement.WhereClauseOperatorEnum#EQUALS}
+	 * or
+	 * {@link ca.uhn.fhir.jpa.fql.parser.HfqlStatement.WhereClauseOperatorEnum#IN}
+	 */
+	private static boolean isDataValueWhereClause(HfqlStatement.WhereClause next) {
+		return next.getOperator() == HfqlStatement.WhereClauseOperatorEnum.EQUALS
+				|| next.getOperator() == HfqlStatement.WhereClauseOperatorEnum.IN;
+	}
+
 	@SuppressWarnings("unchecked")
 	static Comparator<IHfqlExecutionResult.Row> newRowComparator(int columnIndex, HfqlDataTypeEnum dataType) {
 		return Comparator.comparing(new RowValueExtractor(columnIndex, dataType));

hapi-fhir-jpaserver-hfql/src/main/java/ca/uhn/fhir/jpa/fql/parser/HfqlStatement.java

@@ -26,6 +26,7 @@ import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
@@ -303,6 +304,11 @@ public class HfqlStatement implements IModelJson {
 			return myRight;
 		}
 
+		public void setRight(String... theValues) {
+			myRight.clear();
+			myRight.addAll(Arrays.asList(theValues));
+		}
+
 		public void addRight(String theRight) {
 			myRight.add(theRight);
 		}

hapi-fhir-jpaserver-hfql/src/test/java/ca/uhn/fhir/jpa/fql/executor/HfqlExecutorTest.java

@@ -924,7 +924,7 @@ public class HfqlExecutorTest {
 	}
 
 	@Test
-	public void testWhere_Id_In_CommaList() {
+	public void testWhere_Id_In_CommaList_SearchMatch() {
 		IFhirResourceDao<Observation> patientDao = initDao(Observation.class);
 		Observation resource = new Observation();
 		resource.getMeta().setVersionId("5");
@@ -961,6 +961,34 @@ public class HfqlExecutorTest {
 		assertEquals("Patient/456", ((TokenParam) map.get("_id").get(0).get(1)).getValue());
 	}
 
+	@Test
+	public void testWhere_FhirPathElevatedToSearchParam_Id_Equals() {
+		IFhirResourceDao<Patient> patientDao = initDao(Patient.class);
+		when(patientDao.search(any(), any())).thenReturn(createProviderWithSomeSimpsonsAndFlanders());
+
+		String statement = """
+					select id
+					from Patient
+					where	id IN ('HOMER0', 'HOMER1')
+			""";
+
+		IHfqlExecutionResult result = myHfqlExecutor.executeInitialSearch(statement, null, mySrd);
+
+		assertTrue(result.hasNext());
+		List<Object> nextRow = result.getNextRow().getRowValues();
+		assertEquals("HOMER0", nextRow.get(0));
+
+		verify(patientDao, times(1)).search(mySearchParameterMapCaptor.capture(), any());
+		SearchParameterMap map = mySearchParameterMapCaptor.getValue();
+		assertEquals(1, map.get("_id").size());
+		assertEquals(2, map.get("_id").get(0).size());
+		assertNull(((TokenParam) map.get("_id").get(0).get(0)).getSystem());
+		assertEquals("HOMER0", ((TokenParam) map.get("_id").get(0).get(0)).getValue());
+		assertNull(((TokenParam) map.get("_id").get(0).get(1)).getSystem());
+		assertEquals("HOMER1", ((TokenParam) map.get("_id").get(0).get(1)).getValue());
+	}
+
+
 	@Test
 	public void testSearch_QualifiedSelect() {
 		IFhirResourceDao<Patient> patientDao = initDao(Patient.class);
@@ -1211,6 +1239,20 @@ public class HfqlExecutorTest {
 		assertErrorMessage(result, "HAPI-2413: search_match function requires 2 arguments");
 	}
 
+	@Test
+	public void testError_InvalidWhereParameter() {
+		initDao(Patient.class);
+
+		String input = """
+			select name.family
+			from Patient
+			where Blah = '123'
+			""";
+
+		IHfqlExecutionResult result = myHfqlExecutor.executeInitialSearch(input, null, mySrd);
+		assertErrorMessage(result, "HAPI-2429: Resource type Patient does not have a root element named 'Blah'");
+	}
+
 	@SuppressWarnings("unchecked")
 	private <T extends IBaseResource> IFhirResourceDao<T> initDao(Class<T> theType) {
 		IFhirResourceDao<T> retVal = mock(IFhirResourceDao.class);
@@ -1325,6 +1367,7 @@ public class HfqlExecutorTest {
 	@Nonnull
 	private static Patient createPatientHomerSimpson() {
 		Patient homer = new Patient();
+		homer.setId("HOMER0");
 		homer.getMeta().setVersionId("2");
 		homer.addName().setFamily("Simpson").addGiven("Homer").addGiven("Jay");
 		homer.addIdentifier().setSystem("http://system").setValue("value0");

hapi-fhir-testpage-overlay/src/main/java/ca/uhn/fhir/to/Controller.java

@@ -384,7 +384,7 @@ public class Controller extends BaseController {
 			final ModelMap theModel) {
 		addCommonParamsForHfql(theServletRequest, theRequest, theModel);
 
-		ourLog.info("Executing HFQL query: {}", theHfqlQuery);
+		ourLog.info("Executing HFQL query: {}", theHfqlQuery.replaceAll("\\s+", " "));
 		StopWatch sw = new StopWatch();
 
 		List<List<String>> rows = new ArrayList<>();

hapi-fhir-testpage-overlay/src/main/webapp/WEB-INF/templates/hfql.html

@@ -41,16 +41,7 @@
 
 					<div th:replace="tmpl-banner :: banner"></div>
 
-               <div class="card" style="margin-top: 10px;">
+               <div th:replace="tmpl-hfql-banner-card :: banner"></div>
-                  <h3 class="card-header">HFQL / SQL Execution</h3>
-                  <div class="card-body">
-                     This page can be used to execute queries using the HAPI FHIR Query Language (HFQL),
-                     which is a SQL-like syntax for querying FHIR repositories. Learn more about
-                     the HFQL syntax at:
-                     <a href="https://smilecdr.com/docs/hfql/">https://smilecdr.com/docs/hfql/</a>.
-                     This UI will display a maximum of [[${rowLimit}]] rows.
-                  </div>
-               </div>
 
                <!-- ************************************************ -->
                <!-- ** SQL Editor                                 ** -->
@@ -71,8 +62,6 @@
                      <script th:src="@{/resources/ace-builds/src-min-noconflict/ace.js}"></script>
                      <script>
                         const editor = ace.edit("editor");
-                        editor.setTheme("ace/theme/eclipse");
-                        editor.setTheme("ace/theme/vibrant_ink");
                         editor.setTheme("ace/theme/cobalt");
                         editor.session.setMode("ace/mode/sql");
                         editor.setShowPrintMargin(false);

hapi-fhir-testpage-overlay/src/main/webapp/WEB-INF/templates/tmpl-hfql-banner-card.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html lang="en" xmlns:th="http://www.thymeleaf.org">
+<div class="card" style="margin-top: 10px;" th:fragment="banner">
+   <h3 class="card-header">HFQL / SQL Execution</h3>
+   <div class="card-body">
+      This page can be used to execute queries using the HAPI FHIR Query Language (HFQL),
+      which is a SQL-like syntax for querying FHIR repositories. Learn more about
+      HFQL architecture at <a href="https://smilecdr.com/docs/hfql/">https://smilecdr.com/docs/hfql/</a>, and
+      learn more about the HFQL syntax at <a href="https://smilecdr.com/docs/hfql/sql_syntax.html">https://smilecdr.com/docs/hfql/sql_syntax.html</a>.
+      This UI will display a maximum of [[${rowLimit}]] rows.
+   </div>
+</div>