Performance: fivestar_static() queries for current user's vote even if it's not displayed [#410154]

fivestar_static() unconditionally calls the fivestar_get_votes() function, which in turn runs two SQL queries -- one for the average vote and one for the vote that the currently logged in user gave.

It does this even when the user's vote is not displayed anywhere. This could be optimized by only querying for the logged in user's vote if that information is going to be displayed.

Comment	File	Size	Author
#7	fivestar_get_votes_efficiency2.patch	1.65 KB	quicksketch
#7
#6	fivestar_get_votes_efficiency.patch	1.63 KB	quicksketch
#6
#5	410154.patch	1.72 KB	Wesley Tanaka
#5
#2	410154.patch	2.55 KB	Wesley Tanaka
#2
#3	410154.patch	2.41 KB	Wesley Tanaka
#3
#1	410154.patch	2.13 KB	Wesley Tanaka
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Wesley Tanaka CreditAttribution: Wesley Tanaka commented 22 March 2009 at 15:36

File	Size
410154.patch	2.13 KB

This patch fixes the bug, removing the unnecessary SQL query in the case where the user's vote is not displayed

File	Size
410154.patch	2.55 KB

Comment #3

Wesley Tanaka CreditAttribution: Wesley Tanaka commented 22 March 2009 at 15:52

File	Size
410154.patch	2.41 KB

The first patch did not work as advertised. This one should.

Comment #4

quicksketch

he/him

CreditAttribution: quicksketch commented 29 April 2009 at 20:38

Status:

Active

» Needs work

This patch still isn't quite right because the "dual" display needs the user rating. It looks like this code could be shortened up a bit, since the *only* situation where the user's rating is not needed is if the the display is "average" only.

Comment #5

Wesley Tanaka CreditAttribution: Wesley Tanaka commented 1 May 2009 at 20:29

Version:	6.x-1.13	» 6.x-1.15
Status:	Needs work	» Needs review

File	Size
410154.patch	1.72 KB

Rerolled the patch against 1.15 and incorporated quicksketch's suggestion in comment #4

Comment #6

quicksketch

he/him

CreditAttribution: quicksketch commented 1 July 2009 at 03:00

Category:	bug	» task
Status:	Needs review	» Needs work

File	Size
fivestar_get_votes_efficiency.patch	1.63 KB

Heh, well there's *still* one use-case that this breaks. The user's vote is still necessary if using any of the display texts except "none" and "average". I rerolled and applied the attached patch, which should satisfy the performance improvement you were hoping for.