google / android-fhir

The Android FHIR SDK is a set of Kotlin libraries for building offline-capable, mobile-first healthcare applications using the HL7® FHIR® standard on Android.
https://google.github.io/android-fhir/
Apache License 2.0
465 stars 246 forks source link

Improve search speed #2553

Closed MJ1998 closed 2 weeks ago

MJ1998 commented 1 month ago

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #2547 As commented in the above issue, this change reduces time from ~25 seconds to few milliseconds (7k patients).

Description Remove extra JOIN clause (a.resourceType = b.resourceType). This clause is not needed as we already filter the resourceType in WHERE clause.

Sample DSL search:-

fhirEngine.search<Patient> {
        sort(Patient.NAME, Order.ASCENDING)
        count = PaginationConstant.DEFAULT_PAGE_SIZE
        from = currentPage * PaginationConstant.DEFAULT_PAGE_SIZE
      }

Query before this change:-

SELECT a.resourceUuid, a.serializedResource
FROM ResourceEntity a
         LEFT JOIN StringIndexEntity b
                   ON a.resourceType = b.resourceType AND a.resourceUuid = b.resourceUuid AND b.index_name = 'name'
WHERE a.resourceType = 'Patient'
ORDER BY b.index_value ASC
LIMIT 20 OFFSET 0

Query after this change:-

SELECT a.resourceUuid, a.serializedResource
FROM ResourceEntity a
         LEFT JOIN StringIndexEntity b
                   ON a.resourceUuid = b.resourceUuid AND b.index_name = 'name'
WHERE a.resourceType = 'Patient'
ORDER BY b.index_value ASC
LIMIT 20 OFFSET 0

Notice that a.resourceType = b.resourceType clause is not present after ON.

Alternative(s) considered None

Type Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)

Screenshots (if applicable)

Checklist

jingtang10 commented 3 weeks ago

great pr thanks @MJ1998!

one thing i want to clarify, do you know if this column in the string index table (and similarly in other index tables) https://github.com/google/android-fhir/blob/5efb955619cc1f0ca54e0ce4bee9b7c69360d67b/engine/src/main/java/com/google/android/fhir/db/impl/entities/StringIndexEntity.kt#L49C26-L50C34 can be removed?

chandrashekar-s commented 2 weeks ago

Sorry for commenting late here, based on the solution I see that we are going with the assumption that Resource Id is unique across all resource types. But, as per the FHIR specification The logical id is unique within the space of all resources of the same type on the same server. Just raising it here so that we are aware of this.

jingtang10 commented 2 weeks ago

Sorry for commenting late here, based on the solution I see that we are going with the assumption that Resource Id is unique across all resource types. But, as per the FHIR specification The logical id is unique within the space of all resources of the same type on the same server. Just raising it here so that we are aware of this.

In our query we already use a uuid to join the two tables so the resource type join is superfluous.

The uuid is not the resource logical id. It's an id we create for each resource.

chandrashekar-s commented 2 weeks ago

Sorry for commenting late here, based on the solution I see that we are going with the assumption that Resource Id is unique across all resource types. But, as per the FHIR specification The logical id is unique within the space of all resources of the same type on the same server. Just raising it here so that we are aware of this.

In our query we already use a uuid to join the two tables so the resource type join is superfluous.

The uuid is not the resource logical id. It's an id we create for each resource.

Oh I see. I had a look at the ResourceEntity table and the uuid field is unique and there is also a ResourceId column which I asume is the logical id. Thanks for clarifying.