-
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
Add index to DateTimeIndexEntity table index_from column #1964
Add index to DateTimeIndexEntity table index_from column #1964
Conversation
Signed-off-by: Elly Kitoto <[email protected]>
- With unmerged PR #1 - With unmerged PR google#1917 - With unmerged PR google#1978 - With unmerged PR google#1964
- With unmerged PR #1 - With unmerged PR google#1917 - With unmerged PR google#1978 - With unmerged PR google#1964
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ellykits Can you please confirm that the index works as expected while sorting data.
During the testing on my end, I found that the query plan seems to skip this index (_lastUpdatedAt) when used to sort data. But it looks like even though the query plan seems to skip it, having this index makes the query faster See _lastUpdatedAt Performance Tests discussion. |
I'm okay with merging this @jingtang10 |
@ellykits Can you please update the branch to latest. |
Got the following response from this query that searches & sort Groups resources to confirm @aditya-07's observation. EXPLAIN QUERY PLAN SELECT a.serializedResource
FROM ResourceEntity a
LEFT JOIN DateIndexEntity b
ON a.resourceType = b.resourceType AND a.resourceUuid = b.resourceUuid AND b.index_name = '_lastUpdated'
LEFT JOIN DateTimeIndexEntity c
ON a.resourceType = c.resourceType AND a.resourceUuid = c.resourceUuid AND c.index_name = '_lastUpdated'
WHERE a.resourceType = 'Group'
AND a.resourceUuid IN (SELECT resourceUuid
FROM TokenIndexEntity
WHERE resourceType = 'Group'
AND index_name = 'type'
AND (index_value = 'person' AND IFNULL(index_system, '') = 'http://hl7.org/fhir/group-type'))
AND a.resourceUuid IN (SELECT resourceUuid
FROM TokenIndexEntity
WHERE resourceType = 'Group'
AND index_name = 'code'
AND (index_value = '35359004' AND IFNULL(index_system, '') = 'https://www.snomed.org'))
ORDER BY b.index_from DESC, c.index_from DESC
LIMIT 10 OFFSET 0; 12 0 0 SEARCH a USING INDEX index_ResourceEntity_resourceType_resourceId (resourceType=?)
20 0 0 LIST SUBQUERY 1
23 20 0 SEARCH TokenIndexEntity USING INDEX index_TokenIndexEntity_resourceType_index_name_index_system_index_value (resourceType=? AND index_name=?)
46 0 0 LIST SUBQUERY 2
49 46 0 SEARCH TokenIndexEntity USING INDEX index_TokenIndexEntity_resourceType_index_name_index_system_index_value (resourceType=? AND index_name=?)
69 0 0 SEARCH b USING INDEX index_DateIndexEntity_resourceType_index_name_index_from_index_to (resourceType=? AND index_name=?) LEFT-JOIN
80 0 0 SEARCH c USING INDEX index_DateTimeIndexEntity_resourceType_index_name_index_from_index_to (resourceType=? AND index_name=?) LEFT-JOIN
112 0 0 USE TEMP B-TREE FOR ORDER BY |
- With unmerged PR #1 - With unmerged PR google#1917 - With unmerged PR google#1978 - With unmerged PR google#1964 - With unmerged PR google#1963 - With unmerged PR google#1907
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
engine/src/main/java/com/google/android/fhir/db/impl/entities/DateTimeIndexEntity.kt
Outdated
Show resolved
Hide resolved
Hi @ellykits - can you fix this test: |
- With unmerged PR #1 - With unmerged PR google#1917 - With unmerged PR google#1978 - With unmerged PR google#1964 - With unmerged PR google#1907
Signed-off-by: Elly Kitoto <[email protected]>
Head branch was pushed to by a user without write access
engine/src/androidTest/java/com/google/android/fhir/db/impl/ResourceDatabaseMigrationTest.kt
Outdated
Show resolved
Hide resolved
engine/schemas/com.google.android.fhir.db.impl.ResourceDatabase/2.json
Outdated
Show resolved
Hide resolved
engine/schemas/com.google.android.fhir.db.impl.ResourceDatabase/2.json
Outdated
Show resolved
Hide resolved
engine/schemas/com.google.android.fhir.db.impl.ResourceDatabase/2.json
Outdated
Show resolved
Hide resolved
Signed-off-by: Elly Kitoto <[email protected]>
engine/src/androidTest/java/com/google/android/fhir/db/impl/ResourceDatabaseMigrationTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a small comment, otherwise looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #1944
Description
Index the column that contains _lastUpdatedAt date to improve performance on search/sorting
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
No alternative considered
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.