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.

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

Change GetClanDetailsQuery #59

Merged
merged 5 commits into from
Jun 4, 2020
Merged

Conversation

victor-carvalho
Copy link
Contributor

@victor-carvalho victor-carvalho commented Jun 3, 2020

This very little change in the query has a huge impact on performance in postgresql as seen below. The reason is that the comparison using < and > can better use the index and the comparison using <> needs to do a full index scan.

This is the QUERY PLAN executed using EXPLAIN ANALYZE on production DB before the change:
Screen Shot 2020-06-03 at 17 52 49

As you can see in the image the execution time takes almost 2s because on step Subquery Scan "SELECT* 2" it needs to do a scan to find the matches and the table has 3534350 entries.

This is the QUERY PLAN after the change:
Screen Shot 2020-06-03 at 17 54 47

Now the execution time is 32ms because it doesn't need to do a full index scan and for this example it runs more than 20 times faster.

@leohahn
Copy link

leohahn commented Jun 3, 2020

lgtm

Copy link
Contributor

@andrehp andrehp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite an impressive improvement from this change, but I think you added the wrong picture as the second image.

models/clan.go Outdated
@@ -805,7 +805,7 @@ func GetClanDetails(db DB, gameID string, clan *Clan, maxClansPerPlayer int, opt
UNION ALL (
SELECT *
FROM memberships im
WHERE im.clan_id=$2 AND im.deleted_at=0 AND im.approved=false AND im.denied=false AND im.banned=false AND im.requestor_id<>im.player_id
WHERE im.clan_id=$2 AND im.deleted_at=0 AND im.approved=false AND im.denied=false AND im.banned=false AND (im.requestor_id>im.player_id OR im.requestor_id<im.player)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing _id at the end of your query

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to see you around @andrehp! thanks for reviewing!

@victor-carvalho
Copy link
Contributor Author

I fixed the image :)

@felipejfc
Copy link
Contributor

any reason not to merge this @leohahn ?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 61.243% when pulling e53a12c on victor-carvalho:master into 9101efb on topfreegames:master.

@leohahn leohahn merged commit 329c3ff into topfreegames:master Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants