Tune user_access [#181284]

The query in user_access() uses DISTINCT which is superfluous. On any properly setup Drupal site any two different roles will have different permissions.

The DISTINCT also makes the query slow since the permission strings can be quite long. The query shows up in the slow query log on drupal.org:

### 74 Queries
### Taking 16 to 21 seconds to complete

SELECT DISTINCT(p.perm) FROM role r INNER JOIN permission p ON p.rid = r.rid WHERE r.rid IN (XXX);

This is on the new, fast server, there have been more queries on the old one.

Even if there should be two roles with identical permissions, the new query won't break anything. The only thing which will happen is that the $perm static array will gain a few hundred bytes. This might slow down the strpos check later, but I'd argue that a site shouldn't have two roles with identical permissions.

Comment	File	Size	Author
#10	tune_user_access-181284-10.patch	1.4 KB	chx
#7	tune_user_access-181284-7.patch	1.15 KB	chx
#6	test_7.txt	2.26 KB	killes@www.drop.org
#5	tune_user_access_1.patch	1.34 KB	killes@www.drop.org
#4	tune_user_access_0.patch	1.33 KB	killes@www.drop.org
#3	tune_user_access.patch	2.93 KB	killes@www.drop.org
#2	test_6.txt	2.13 KB	killes@www.drop.org
	no_distinct.patch	991 bytes	killes@www.drop.org

Comments

Comment #1

robertdouglass commented 6 October 2007 at 12:04

This raises several observations and questions.

First, I'm inclined to agree that the distinct can be dropped. If we have more than one row per rid in the permissions table, is that not an application error somewhere else? Even if we did, isn't that role entitled to all of those permissions? So the perm strings would be concatenated, split into an array, and an in_array would be done on it. Not catastrophic.

The first question I have when looking, though, is what does permission.tid do?
The second question I have is why are these two separate tables? Why is perm not a column of role?

Comment #2

killes@www.drop.org commented 6 October 2007 at 13:01

Title:

Remove DISTINCT in query

» Tune user_access

Status	File	Size
new	test_6.txt	2.13 KB

tid can be dropped, not sure why it hasn't been. I've no idea why permissions is an extra table.

I've tuned the function some more. See attached test script.

I've found that using in_array is much faster.

Comment #3

killes@www.drop.org commented 6 October 2007 at 13:03

Status	File	Size
new	tune_user_access.patch	2.93 KB

Here's the improved patch.

Comment #4

killes@www.drop.org commented 6 October 2007 at 13:08

Status	File	Size
new	tune_user_access_0.patch	1.33 KB

More simplification.

Comment #5

killes@www.drop.org commented 6 October 2007 at 15:51

Status	File	Size
new	tune_user_access_1.patch	1.34 KB

The previous patch wasn't quite right and due to this in_array is actually faster than the current approach but if we restrict strpos to only act on unique permissions, we gain more.