Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Include index_name column in index for tables used in sorting #2753

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Include index_name column in index for tables used in sorting #2753

wants to merge 4 commits into from

Conversation

LZRS
Copy link
Collaborator

@LZRS LZRS commented Dec 6, 2024 •
edited
Loading

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Improves performance for queries that may involve sorting with search params

Fixes #2758

Description
Clear and concise code change description.

Alternative(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

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

@LZRS LZRS mentioned this pull request Dec 6, 2024
Closed
7 tasks
@LZRS
Copy link
Collaborator Author

LZRS commented Dec 19, 2024 •
edited
Loading

Given query

SELECT a.resourceId
FROM ResourceEntity a
         LEFT JOIN DateIndexEntity b
                   ON a.resourceUuid = b.resourceUuid AND b.index_name = '_lastUpdated'
         LEFT JOIN DateTimeIndexEntity c
                   ON a.resourceUuid = c.resourceUuid AND c.index_name = '_lastUpdated'
WHERE a.resourceType = 'Encounter'
GROUP BY a.resourceUuid
HAVING MAX(IFNULL(b.index_from, 0) + IFNULL(c.index_from, 0)) >= -9223372036854775808
ORDER BY IFNULL(b.index_from, -9223372036854775808) DESC, IFNULL(c.index_from, -9223372036854775808) DESC;

previous query generated was

QUERY PLAN
|--SEARCH a USING INDEX index_ResourceEntity_resourceType_resourceId (resourceType=?)
|--SEARCH b USING INDEX index_DateIndexEntity_resourceUuid (resourceUuid=?) LEFT-JOIN
|--SEARCH c USING INDEX index_DateTimeIndexEntity_resourceUuid (resourceUuid=?) LEFT-JOIN
|--USE TEMP B-TREE FOR GROUP BY
`--USE TEMP B-TREE FOR ORDER BY

this changes to

QUERY PLAN
|--SEARCH a USING INDEX index_ResourceEntity_resourceType_resourceId (resourceType=?)
|--SEARCH b USING COVERING INDEX index_DateIndexEntity_resourceUuid_index_name_index_from (resourceUuid=? AND index_name=?) LEFT-JOIN
|--SEARCH c USING COVERING INDEX index_DateTimeIndexEntity_resourceUuid_index_name_index_from (resourceUuid=? AND index_name=?) LEFT-JOIN
|--USE TEMP B-TREE FOR GROUP BY
`--USE TEMP B-TREE FOR ORDER BY

And testing in a database with 166293 resources and 137517 encounters, the previous query plan took

Run Time: real 4.606 user 0.675157 sys 0.889591

while the updated takes

Run Time: real 4.120 user 0.533021 sys 0.707809

@LZRS
Copy link
Collaborator Author

LZRS commented Dec 19, 2024

In a database with 125206 resources and 109654 tasks,

query

SELECT a.resourceId
FROM ResourceEntity a
         LEFT JOIN DateIndexEntity b
                   ON a.resourceUuid = b.resourceUuid AND b.index_name = '_lastUpdated'
         LEFT JOIN DateTimeIndexEntity c
                   ON a.resourceUuid = c.resourceUuid AND c.index_name = '_lastUpdated'
WHERE a.resourceType = 'Task'
GROUP BY a.resourceUuid
HAVING MAX(IFNULL(b.index_from, 0) + IFNULL(c.index_from, 0)) >= -9223372036854775808
ORDER BY IFNULL(b.index_from, -9223372036854775808) DESC, IFNULL(c.index_from, -9223372036854775808) DESC;

previously took

Run Time: real 3.124 user 0.451723 sys 0.499627

compared to

Run Time: real 3.032 user 0.383676 sys 0.493447

@MJ1998
Copy link
Collaborator

MJ1998 commented Dec 20, 2024

@LZRS is this intended to be a draft PR ?
Can you attach an issue with it ?

@LZRS LZRS marked this pull request as ready for review December 20, 2024 14:02
@LZRS LZRS requested a review from a team as a code owner December 20, 2024 14:02
@LZRS LZRS requested a review from MJ1998 December 20, 2024 14:02
@LZRS
Copy link
Collaborator Author

LZRS commented Dec 20, 2024 •
edited
Loading

@LZRS is this intended to be a draft PR ? Can you attach an issue with it ?

I've added this issue. I've also marked the PR's as ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MJ1998 MJ1998 Awaiting requested review from MJ1998 MJ1998 is a code owner automatically assigned from google/android-fhir-reviewers

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update IndexEntity tables used in sorting to include index_name and index_value in their resourceUuid index
2 participants
@LZRS @MJ1998