-
Notifications
You must be signed in to change notification settings - Fork 296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds resourceUuid
to the multi-column index in ReferenceIndexEntity
#2745
base: master
Are you sure you want to change the base?
Conversation
e464ce9
to
bafa004
Compare
The order of results in include/revInclude seems to be no longer predictable when searching using resourceUuid index the resourceUuids are randomly generated, also saved in the db as blob hence ordered by byte representation of the resourceUuid
engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
Outdated
Show resolved
Hide resolved
Previously, the sql query SELECT *
FROM (SELECT rie.index_name, rie.index_value, re.serializedResource
FROM ResourceEntity re
JOIN ReferenceIndexEntity rie
ON re.resourceUuid = rie.resourceUuid
WHERE rie.resourceType = 'Encounter'
AND rie.index_name = 'service-provider'
AND rie.index_value IN ('Organization/2c29c69f-c2d1-463f-a4b2-d90a5c2fd05d')
AND re.resourceType = 'Encounter') generated the query plan
Testing with a database that has 166293 resources and 137517 encounters, the above query could take
returning 137517 rows The changes in this PR, would generate query plan
using a covering index Testing with a database that has 166293 resources and 137517 encounters, the above query takes around
returning 137517 rows |
I thought adding a non-filter key to the multi-column index would not improve the performance. This is a surprise for me. I have more questions around why our query is like this
Since our |
The non-filter column
Yeah, I agree. For this type of query, it makes sense to remove the |
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #[issue number]
Description
Adds
resourceUuid
to the multi-column index in ReferenceIndexEntity table to make it a covering indexAlternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.