Problem/Motivation

lacksPaginationInContext() in modules/audit_performance/src/Plugin/AuditAnalyzer/PerformanceAnalyzer.php reports an entity query or a \Drupal::database()->select() when an ->execute( appears within the next 15 lines and no ->range( / ->pager( is found there. The only exemption is a class that extends EntityListBuilder.

Verified on current 1.x:

- Count queries are reported (\Drupal::entityQuery('node')->accessCheck(TRUE)->count()->execute(), ->countQuery()->execute()->fetchField()), although they return a single row.
- Queries restricted to one entity by its ID or UUID are reported (->condition('nid', $nid), ->condition('n.nid', $nid) on node_field_data, ->condition('uuid', $uuid), ->condition('nid', $ids, 'IN')).
- The 15-line window does not stop at the query's execute(): a range() of the NEXT query in those lines hides a real unpaginated query (false negative).

Some exemptions proposed in earlier reports were checked and rejected:

- Skipping queries that end in ->fetchField() / ->fetchAssoc() / ->fetchObject(): the MySQL driver uses buffered queries (PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => TRUE in core's mysql Connection), so the whole result set is transferred to PHP memory even if only the first row is read.
- Guessing the primary key from the column name (nid, uid, tid, vid...): uid on a node query and vid on a term query are not the entity ID and select many rows.
- Skipping whole classes or functions (queue workers, batch callbacks, Search API processors, migrate plugins): those are exactly where loading a full table hurts; intentional cases can be acknowledged with an audit-ignore marker.

Steps to reproduce

1. Add $count = \Drupal::entityQuery('node')->accessCheck(TRUE)->count()->execute(); and $ids = \Drupal::entityQuery('node')->accessCheck(TRUE)->condition('nid', $nid)->execute(); to a custom module.
2. Run drush audit:run performance --format=json.
3. QUERY_NO_RANGE is reported on both lines.

Proposed resolution

In lacksPaginationInContext():
- Only look at the query up to the end of the line of its first ->execute(), so a range() of a later query is not taken into account.
- Skip count queries (->count() or ->countQuery()).
- Skip queries with an equality ('=' or default) or 'IN' condition on the entity's ID or UUID key, taken from the entity type definition (new isQueryBoundedById() helper, using the entity type manager injected in #3630468). The entity type is resolved from entityQuery('type'), from getStorage('type')->getQuery() or a storage variable or property assigned from getStorage('type') (the resolver of #3630468, extracted to resolveStorageEntityTypeId()), and for database selects from the entity base or data table they select from (an optional table alias prefix is accepted). Tables that are not entity tables, and unresolved queries, are still reported.

Remaining tasks

- Implement the window limit, the count skip and isQueryBoundedById(); extract resolveStorageEntityTypeId() from isConfigEntityStorageCall().
- Unit test with a mocked entity type manager and a data provider. Reported: bundle condition, node author (uid), term vocabulary (vid), ID with a '>' operator, database select by author, the first row of an unbounded select with fetchField(), an ID condition on a table that is not an entity table, and a query whose only range() belongs to the next query. Not reported: range(), entity count, database count, node ID, UUID with '=', ID list with 'IN' through getStorage()->getQuery(), term ID through a storage variable, database select on node_field_data by n.nid.

User interface changes

Fewer QUERY_NO_RANGE / DB_SELECT_NO_LIMIT warnings on bounded queries; unpaginated queries followed by another query with range() are now reported.

API changes

None.

Data model changes

None.

Comments

trebormc created an issue. See original summary.

  • trebormc committed 21ffc090 on 1.x
    Issue #3630471 by trebormc: Skip count queries and queries restricted to...
trebormc’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.