Fix failing test for disjoint filters with nested query

google · Dec 18, 2024 · 3d38081 · 3d38081
1 parent 6b2c021
commit 3d38081
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 87 deletions.
diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
@@ -2809,78 +2809,79 @@ class DatabaseImplTest {
   }
 
   @Test
-  fun test_search_multiple_param_disjunction_covid_immunization_records() = runBlocking {
-    val resources =
-      listOf(
-        Immunization().apply {
-          id = "immunization-1"
-          vaccineCode =
-            CodeableConcept(
-              Coding(
-                "http://id.who.int/icd11/mms",
-                "XM1NL1",
-                "COVID-19 vaccine, inactivated virus",
-              ),
-            )
-          status = Immunization.ImmunizationStatus.COMPLETED
-        },
-        Immunization().apply {
-          id = "immunization-2"
-          vaccineCode =
-            CodeableConcept(
-              Coding(
-                "http://id.who.int/icd11/mms",
-                "XM5DF6",
-                "COVID-19 vaccine, live attenuated virus",
-              ),
-            )
-          status = Immunization.ImmunizationStatus.COMPLETED
-        },
-        Immunization().apply {
-          id = "immunization-3"
-          vaccineCode =
-            CodeableConcept(
-              Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based"),
-            )
-          status = Immunization.ImmunizationStatus.COMPLETED
-        },
-        Immunization().apply {
-          id = "immunization-4"
-          vaccineCode =
-            CodeableConcept(
-              Coding(
-                "http://hl7.org/fhir/sid/cvx",
-                "140",
-                "Influenza, seasonal, injectable, preservative free",
-              ),
-            )
-          status = Immunization.ImmunizationStatus.COMPLETED
-        },
-      )
+  fun test_search_multiple_param_disjunction_covid_immunization_records() {
+    runBlocking {
+      val resources =
+        listOf(
+          Immunization().apply {
+            id = "immunization-1"
+            vaccineCode =
+              CodeableConcept(
+                Coding(
+                  "http://id.who.int/icd11/mms",
+                  "XM1NL1",
+                  "COVID-19 vaccine, inactivated virus",
+                ),
+              )
+            status = Immunization.ImmunizationStatus.COMPLETED
+          },
+          Immunization().apply {
+            id = "immunization-2"
+            vaccineCode =
+              CodeableConcept(
+                Coding(
+                  "http://id.who.int/icd11/mms",
+                  "XM5DF6",
+                  "COVID-19 vaccine, live attenuated virus",
+                ),
+              )
+            status = Immunization.ImmunizationStatus.COMPLETED
+          },
+          Immunization().apply {
+            id = "immunization-3"
+            vaccineCode =
+              CodeableConcept(
+                Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based"),
+              )
+            status = Immunization.ImmunizationStatus.COMPLETED
+          },
+          Immunization().apply {
+            id = "immunization-4"
+            vaccineCode =
+              CodeableConcept(
+                Coding(
+                  "http://hl7.org/fhir/sid/cvx",
+                  "140",
+                  "Influenza, seasonal, injectable, preservative free",
+                ),
+              )
+            status = Immunization.ImmunizationStatus.COMPLETED
+          },
+        )
 
-    database.insert(*resources.toTypedArray())
+      database.insert(*resources.toTypedArray())
 
-    val result =
-      database.search<Immunization>(
-        Search(ResourceType.Immunization)
-          .apply {
-            filter(
-              Immunization.VACCINE_CODE,
-              { value = of(Coding("http://id.who.int/icd11/mms", "XM1NL1", "")) },
-            )
+      val result =
+        database.search<Immunization>(
+          Search(ResourceType.Immunization)
+            .apply {
+              filter(
+                Immunization.VACCINE_CODE,
+                { value = of(Coding("http://id.who.int/icd11/mms", "XM1NL1", "")) },
+              )
 
-            filter(
-              Immunization.VACCINE_CODE,
-              { value = of(Coding("http://id.who.int/icd11/mms", "XM5DF6", "")) },
-            )
-            operation = Operation.OR
-          }
-          .getQuery(),
-      )
+              filter(
+                Immunization.VACCINE_CODE,
+                { value = of(Coding("http://id.who.int/icd11/mms", "XM5DF6", "")) },
+              )
+              operation = Operation.OR
+            }
+            .getQuery(),
+        )
 
-    assertThat(result.map { it.resource.vaccineCode.codingFirstRep.code })
-      .containsExactly("XM1NL1", "XM5DF6")
-      .inOrder()
+      assertThat(result.map { it.resource.vaccineCode.codingFirstRep.code })
+        .containsExactly("XM1NL1", "XM5DF6")
+    }
   }
 
   @Test

diff --git a/engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt b/engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt
@@ -364,17 +364,13 @@ internal fun Search.getQuery(
   val sortArgs = join.args
 
   val filterQuery = getFilterQueries()
-  val filterQueryStatement =
-    filterQuery.joinToString(separator = "${operation.logicalOperator} ") {
-      //  spotless:off
-    """
-      a.resourceUuid IN (
-      ${it.query}
-      )
-      
-      """.trimIndent()
-    //  spotless:on
+  val filterQueryJoinOperator =
+    when (operation) {
+      Operation.OR -> "\nUNION\n"
+      Operation.AND -> "\nINTERSECT\n"
     }
+  val filterQueryStatement =
+    filterQuery.joinToString(separator = filterQueryJoinOperator) { it.query.trimIndent() }
   val filterQueryArgs = filterQuery.flatMap { it.args }
 
   var limitStatement = ""
@@ -398,8 +394,14 @@ internal fun Search.getQuery(
   val filterStatement =
     listOf(filterQueryStatement, nestedQueryFilterStatement)
       .filter { it.isNotBlank() }
-      .joinToString(separator = " AND ")
-      .ifBlank { "a.resourceType = ?" }
+      .joinToString(separator = "\nINTERSECT\n")
+      .takeIf { it.isNotBlank() }
+      ?.let { """a.resourceUuid IN (
+          $it
+          )
+          """ }
+      ?: "a.resourceType = ?"
+
   val filterArgs = (filterQueryArgs + nestedQueryFilterArgs).ifEmpty { listOf(type.name) }
 
   val whereArgs = mutableListOf<Any>()

diff --git a/engine/src/test/java/com/google/android/fhir/search/SearchTest.kt b/engine/src/test/java/com/google/android/fhir/search/SearchTest.kt
@@ -1872,7 +1872,7 @@ class SearchTest {
   }
 
   @Test
-  fun search_patient_multiple_given_disjoint_has_condition_diabetes() {
+  fun search_patient_multiple_given_disjoint_has_condition_diabetes_or_hypertension() {
     val query =
       Search(ResourceType.Patient)
         .apply {
@@ -1881,6 +1881,11 @@ class SearchTest {
               Condition.CODE,
               { value = of(Coding("http://snomed.info/sct", "44054006", "Diabetes")) },
             )
+            filter(
+              Condition.CODE,
+              { value = of(Coding("http://snomed.info/sct", "827069000", "Hypertension stage 1")) },
+            )
+            operation = Operation.OR
           }
 
           filter(
@@ -1907,8 +1912,7 @@ class SearchTest {
         """
         SELECT a.resourceUuid, a.serializedResource
         FROM ResourceEntity a
-        WHERE a.resourceType = ?
-        AND a.resourceUuid IN (
+        WHERE a.resourceUuid IN (
         SELECT resourceUuid FROM StringIndexEntity
         WHERE resourceType = ? AND index_name = ? AND index_value = ?
         UNION
@@ -1920,8 +1924,10 @@ class SearchTest {
         WHERE a.resourceType = ? AND a.resourceId IN (
         SELECT substr(a.index_value, 9)
         FROM ReferenceIndexEntity a
-        WHERE a.resourceType = ? AND a.index_name = ?
-        AND a.resourceUuid IN (
+        WHERE a.index_name = ? AND a.resourceUuid IN (
+        SELECT resourceUuid FROM TokenIndexEntity
+        WHERE resourceType = ? AND index_name = ? AND (index_value = ? AND IFNULL(index_system,'') = ?)
+        UNION
         SELECT resourceUuid FROM TokenIndexEntity
         WHERE resourceType = ? AND index_name = ? AND (index_value = ? AND IFNULL(index_system,'') = ?)
         )
@@ -1934,20 +1940,22 @@ class SearchTest {
     assertThat(query.args)
       .isEqualTo(
         listOf(
-          "Patient",
           "Patient",
           "given",
           "John",
           "Patient",
           "given",
           "Jane",
           "Patient",
-          "Condition",
           "subject",
           "Condition",
           "code",
           "44054006",
           "http://snomed.info/sct",
+          "Condition",
+          "code",
+          "827069000",
+          "http://snomed.info/sct",
         ),
       )
   }