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');

Issue fork drupal-3362407

Command icon 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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Pawelgorski87 created an issue. See original summary.

Pawelgorski87’s picture

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests

Thank 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.

Pawelgorski87’s picture

Version: 9.5.x-dev » 10.2.x-dev

Pawelgorski87’s picture

Status: Needs work » Needs review
Pawelgorski87’s picture

Assigned: Pawelgorski87 » Unassigned

@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

Ghost of Drupal Past’s picture

If it is a performance issue could you perhaps grab the queries and post them with an EXPLAIN? Thanks!

Pawelgorski87’s picture

This is example raw query when you don't add alias in $query->select('node_access'), there is 2x node_access

SELECT 1 AS "expression"
FROM
"node_access" "node_access"
WHERE ("grant_view" >= '1') AND ((("nid" = '22487') AND ("langcode" = 'pl')) OR ("nid" = '0')) AND ((("gid" IN ('0')) AND ("realm" = 'all')) OR (("gid" IN ('2'));

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)

Version: 10.2.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Not super verse in it but #9 mentions running EXPLAIN on the two to compare

smustgrave’s picture

Status: Needs review » Needs work

For #9

Leaving tests tag in case it is determined we will need tests.