Problem/Motivation

The titleQuery() function in web/core/modules/node/src/Plugin/views/argument/Vid.php returns titles for all revisions of all nodes. On a large site with lots of nodes and revisions, this produces a slow query. It also means that the views argument title substitution is wrong.

Proposed resolution

Including a patch to limit Vid->titleQuery() to the provided vid - this mirrors the code in Drupal 7.

This was broken in #2628130: SQL error on revision export from view

CommentFileSizeAuthor
#13 3259671-10.x-13.patch6.72 KBalexpott
#13 3259671-9.x-13.patch6.72 KBalexpott
#13 3259671-9.x-13.test-only.patch6.23 KBalexpott
#11 3259671-11.patch6.72 KBalexpott
#11 3259671-11.test-only.patch6.23 KBalexpott
#2 limitbyvid-3259671-1.patch503 bytes_kash_

Issue fork drupal-3259671

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:

About issue forks

Comments

_KASH_ created an issue. See original summary.

_kash_’s picture

_kash_ commented
Status: Active » Needs review
StatusFileSize
new limitbyvid-3259671-1.patch503 bytes
cilefen’s picture

cilefen commented
Version: 9.2.x-dev » 9.4.x-dev
Issue tags: +Performance
_kash_’s picture

_kash_ commented
Related issues: +#2628130: SQL error on revision export from view

jrglasgow made their first commit to this issue’s fork.

jrglasgow opened merge request !2281

jrglasgow’s picture

jrglasgow commented

This is working for me

krzysztof domański’s picture

krzysztof domański
Location Poland
commented
Status: Needs review » Reviewed & tested by the community
ramarajuk’s picture

ramarajuk commented

This patch works for us.

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 9.5.x-dev » 9.4.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 3259671-11.test-only.patch6.23 KB
new 3259671-11.patch6.72 KB

@jrglasgow it seems your MR is exactly the same as @_KASH_'s patch. So removing credit.

Here's a test - this is not just a performance fix it is also a bug fix. Note that Views in Drupal 7 already does:

$result = db_query("SELECT n.title FROM {node_revision} n WHERE n.vid IN (:vids)", array(':vids' => $this->value));

So it already has the fix or perhaps we broke this when converting to a plugin.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Link to the views Drupal 7 code... https://git.drupalcode.org/project/views/-/blame/7.x-3.x/modules/node/vi...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3259671-9.x-13.test-only.patch6.23 KB
new 3259671-9.x-13.patch6.72 KB
new 3259671-10.x-13.patch6.72 KB

We need different patches on 10.x and 9.x here they all are...

The last submitted patch, 13: 3259671-9.x-13.test-only.patch, failed testing. View results

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Yeah, we broke this in the issue @_KASH_ linked: #2628130: SQL error on revision export from view

Looks like a good fix and test to me.

The last submitted patch, 13: 3259671-9.x-13.test-only.patch, failed testing. View results

The last submitted patch, 13: 3259671-9.x-13.test-only.patch, failed testing. View results

The last submitted patch, 13: 3259671-9.x-13.test-only.patch, failed testing. View results

The last submitted patch, 13: 3259671-9.x-13.patch, failed testing. View results

  • catch committed 334790c on 10.1.x 
    Issue #3259671 by alexpott, _KASH_, Lendude: Slow query in titleQuery...
  • catch committed 7d40953 on 9.4.x
    Issue #3259671 by alexpott, _KASH_, Lendude: Slow query in titleQuery...
  • catch committed e5d6a0c on 9.5.x
    Issue #3259671 by alexpott, _KASH_, Lendude: Slow query in titleQuery...

  • catch committed d02c768 on 10.0.x 
    Issue #3259671 by alexpott, _KASH_, Lendude: Slow query in titleQuery...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, cherry-picked to 10.0.x, then the same for 9.5.x and 9.4.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.