Problem/Motivation
Performance issue during using grands system.
Steps to reproduce
Use grands with a lot numbers of node and users + 3000
Code $query = $this->database->select('node_access'); without alias param
generate db query like this: with 2x node_access
SELECT 1 AS "expression"
FROM
"node_access" "node_access"
WHERE ("grant_view" >= '1') ...
sql can handle it, but the query lengthens very much and makes a lock on the table if we have a large number of users.
Proposed resolution
class NodeGrantDatabaseStorage
line 81
$query = $this->database->select('node_access');
and
line 132
$query = $this->database->select('node_access');
replace on
$query = $this->database->select('node_access', 'na');
Comment | File | Size | Author |
---|---|---|---|
#10 | Screenshot 2024-02-09 at 15.27.45.png | 103.58 KB | Pawelgorski87 |
#10 | Screenshot 2024-02-09 at 15.23.44.png | 121.79 KB | Pawelgorski87 |
#3 | 3362407-nodegrantdatabasestorage-generate-select.patch | 1.09 KB | Pawelgorski87 |
Issue fork drupal-3362407
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
Pawelgorski87 CreditAttribution: Pawelgorski87 as a volunteer and at Droptica commentedComment #4
smustgrave CreditAttribution: smustgrave at Mobomo commentedThank you for reporting.
Can you confirm if this is a D10 problem as well? If so will need a patch that applies to 11.x (Which is the master branch essentially now).
Will also need a test case showing the bug
Thanks.
Comment #5
Pawelgorski87 CreditAttribution: Pawelgorski87 as a volunteer and at Droptica commentedComment #7
Pawelgorski87 CreditAttribution: Pawelgorski87 as a volunteer and at Droptica commentedComment #8
Pawelgorski87 CreditAttribution: Pawelgorski87 as a volunteer and at Droptica commented@smustgrave Sorry, I forgot about that issue, yes it affected all Drupals, select without alias genere 'small' syntax error, sql will handle that but lose performance. Tested on big table grands with +30k items.
It can be hard to test because it works general, but only performance issue, but you can check also all query->select in core has alias, only few missing, probably in 99% cases it doesn't matter.
PR is ready
Comment #9
Ghost of Drupal PastIf it is a performance issue could you perhaps grab the queries and post them with an EXPLAIN? Thanks!
Comment #10
Pawelgorski87 CreditAttribution: Pawelgorski87 as a volunteer and at Droptica commentedThis is example raw query when you don't add alias in $query->select('node_access'), there is 2x node_access
Repeating table names 2x is a syntax error, and SQL must process it. With small queries, it's not noticeable, but we had a lot of users where grants were being generated for about 6000-8000 users at once. Without this change, the MySQL server would crash as it couldn't process the queries. Adding an alias in the code generated the correct syntax, and the problem disappeared.
Not sure it it relevant but i did some small test on small query and there is 0.0001s diffs between queries. (screen)
Comment #12
smustgrave CreditAttribution: smustgrave at Mobomo commentedNot super verse in it but #9 mentions running EXPLAIN on the two to compare
Comment #13
smustgrave CreditAttribution: smustgrave at Mobomo commentedFor #9
Leaving tests tag in case it is determined we will need tests.