The latest_revision ViewsFilter has poor performance

It would be very helpful if you could provide more specific details about your environment, for example the number of records that you have in the base / revision tables (something like: "we have 100.000 entities with 50 revisions each, so 5.000.000 revisions in total").

And can you also post the actual views query which brings down your db?

The majority of the 'is latest revision' code got added / updated in 8.5, so I think it sounds about right that it's running better now in 8.5. I wonder if we could find a way to properly test with 150,000 nodes, each with 10+ revisions.

Found this via #2950869: Entity queries querying the latest revision very slow with lots of revisions, adding related issues etc.

Better issue title per #2950869-17: Entity queries querying the latest revision very slow with lots of revisions.

Hello,

Updating version. I don't know why this issue had been stucked in this version. Still relevant today.

Hello,

Here is a POC/WIP patch.

I guess that the filter core/modules/views/src/Plugin/views/filter/LatestTranslationAffectedRevision.php will also need to be updated.

I will test the patch on a project for which I need performance improvements.

Seeing what the testbot says about the approach in #8.

removed my own comment, see new comment

removed comment

Ok, this not only fixes a performance issue, this fixes the broken behavior of the LatestRevision filter.
The tests need to be updated because the existing core tests are incorrect assertions.

If I'm using the LatestRevision filter I do not expect to see multiple revisions per node in the results, I only expect to see the 'latest' pending revision.

the test case in core is incorrect , it may be passing but the assumptions are incorrect.

So, yes wonderful work on patch 8, I will be reviewing this patch some more. So far it looks like an improvement to me.

the aggregation is #8 is not good, since it prevents the view from selecting other fields that are not included in the `group by` clause .

Added a small patch that solved the performance issues (from 60+ seconds to about 1 second query execution time)
Note that this patches the other filter: LatestTranslationAffectedRevision

The normal left join is replaced with a join to a subquery.

I altered and combined the patches to us where in instead of a join with a > (greater than). In this way it is also possible to add fields to the query.

The queries from my previous patch aren't good. I altered them according to the patch of #18 to work with a subquery but then in an INNER JOIN instead of the LEFT JOIN WHERE vid2 > vid. It seems to me this is done to get the highest vid because when vid2 isn't higher than vid it means for that node it is the MAX(vid) and the inner join reduces the amount or records of the joined query and boosts the performance a bit more.

I've switched this back to filtering by way of WHERE rather than a join. I'm not entirely sure why the switch to using a join was done, but using WHERE and a subquery is massively more performant for me. As in, queries that previously took ~10-15 seconds are now taking just 0.2 seconds. This patch is modeled on the query structure being pursued in #2950869: Entity queries querying the latest revision very slow with lots of revisions.

Well, I can't figure out how to make the WHERE-based solution work for the latest translation affected filter, so I'm reverting that to use the old join-based approach.

Patch #25 solved issue with bad performance on site with >200k revisions.
Before patch: 32 seconds
After patch: 0.4 seconds

+++ b/core/modules/views/src/Plugin/views/filter/LatestRevision.php
@@ -17,6 +18,13 @@
+   * @var \Drupal\Core\Database\Connection
...
+  protected $database;

@@ -40,14 +48,17 @@ class LatestRevision extends FilterPluginBase implements ContainerFactoryPluginI
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, ViewsHandlerManager $join_handler) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, Connection $database, EntityTypeManagerInterface $entity_type_manager, ViewsHandlerManager $join_handler) {

@@ -58,6 +69,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
+      $container->get('database'),

@@ -93,21 +105,10 @@ public function query() {
+    $subquery = $this->database->select($query_base_table, 'base_table');

+++ b/core/modules/views/src/Plugin/views/filter/LatestTranslationAffectedRevision.php
@@ -17,6 +18,13 @@
+  protected $database;

@@ -40,14 +48,17 @@ class LatestTranslationAffectedRevision extends FilterPluginBase implements Cont
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, ViewsHandlerManager $join_handler) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, Connection $database, EntityTypeManagerInterface $entity_type_manager, ViewsHandlerManager $join_handler) {

@@ -58,6 +69,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
+      $container->get('database'),

@@ -93,23 +105,27 @@ public function query() {
+    $subquery = $this->database->select($query_base_table, 'base_table');

instead, introduce a $this->getConnection() method because Sql is not only connection

Here fixed patch #25.
I'm sorry I can't make interdiff right now. I will make it and attach it later.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Reading the comments #29 appears to be addressed in #30.

As a bug believe this will need it's own test case to show the issue is being resolved.

Also posting to #testing to verify that.

I propose also to add new index on node_field_revision table, in my case it extremely increase a performance:
- revision_translation_affected
- nid
- vid

Looks like temporary table constructed in $subquery isn't cached by MariaDB engine. We have much content and more that 100k revisions. Having a dashboard with 4 node revision views (each use "Is Latest Translation Affected Revision" filter) I have the next times of the page render (I make `drush cr` before each load):

Core: 7m 15s
Core+index: 40s
Patch #25: 3m 40s
Patch #25+index: 5s!!!

@smustgrave we can't add tests for query performance, if there's existing test coverage of the latest_revision filter that should be sufficient. Untagging for 'needs tests' but we should verify whether existing tests exist.

@dewalt that seems like a good idea, but should be its own issue, since it'll need an upgrade path etc. Could you open the issue?

Moving back to needs review.

Thanks @catch for cleaning that up.

Opened https://www.drupal.org/project/drupal/issues/3386159 for #33

#30 is all green but issue summary think could use some love so tagging for that. Proposed solution mainly

@smustgrave did you check whether there is existing test coverage of the latest_revision filter?

Sorry back to needs work.

Drupal\Tests\content_moderation\Unit\LatestRevisionCheckTest
Drupal\Tests\Core\Enhancer\EntityRevisionRouteEnhancerTest
Drupal\Tests\Core\ParamConverter\EntityRevisionParamConverte
Drupal\Tests\migrate\Unit\destination\EntityRevisionTest
Drupal\Tests\node\Unit\Plugin\views\field\RevisionLinkTest
Drupal\Tests\node\Unit\Plugin\views\field\RevisionLinkDelete
Drupal\Tests\node\Unit\Plugin\views\field\RevisionLinkRevert
Drupal\Tests\block_content\Kernel\Views\RevisionRelationship
Drupal\Tests\book\Kernel\BookPendingRevisionTest
Drupal\Tests\block_content\Kernel\Views\RevisionUserTest
Drupal\Tests\content_moderation\Kernel\DefaultRevisionStateTest
Drupal\KernelTests\Core\Entity\ContentEntityNonRevisionableF
Drupal\KernelTests\Core\Entity\EntityNonRevisionableTranslat
Drupal\KernelTests\Core\Entity\EntityRevisionsTest
Drupal\KernelTests\Core\Entity\EntityRevisionTranslationTest
Drupal\KernelTests\Core\Entity\RevisionableContentEntityBase
Drupal\KernelTests\Core\Entity\RevisionRouteProviderTest
Drupal\Tests\jsonapi\Kernel\Revisions\VersionNegotiatorTest
Drupal\Tests\media\Kernel\Views\RevisionUserTest
Drupal\Tests\migrate\Kernel\Plugin\EntityRevisionTest
Drupal\Tests\node\Kernel\Migrate\d6\MigrateNodeRevisionTest
Drupal\Tests\node\Kernel\Views\ArgumentNodeRevisionIdTest
Drupal\Tests\node\Kernel\Views\FilterUidRevisionTest
Drupal\Tests\node\Kernel\Views\ArgumentUidRevisionTest
Drupal\Tests\node\Kernel\Views\RevisionRelationshipsTest
Drupal\Tests\node\Kernel\Views\RevisionUidTest
Drupal\Tests\node\Kernel\Migrate\d7\MigrateNodeRevisionTest
Drupal\Tests\taxonomy\Kernel\PendingRevisionTest
Drupal\Tests\node\Kernel\Views\RevisionCreateTimestampTest
Drupal\Tests\views\Kernel\Entity\LatestRevisionFilterTest
Drupal\Tests\block_content\Functional\BlockContentRevisionDe
Drupal\Tests\block_content\Functional\BlockContentRevisionsT
Drupal\Tests\block_content\Functional\BlockContentRevisionRe
Drupal\Tests\block_content\Functional\BlockContentRevisionVe
Drupal\FunctionalTests\Entity\RevisionRouteProviderTest
Drupal\FunctionalTests\Entity\RevisionViewTest
Drupal\Tests\system\Functional\Entity\EntityRevisionsTest
Drupal\Tests\file\Functional\FileFieldRevisionTest
Drupal\FunctionalTests\Entity\RevisionDeleteFormTest
Drupal\FunctionalTests\Entity\RevisionRevertFormTest
Drupal\FunctionalTests\Entity\RevisionVersionHistoryTest
Drupal\Tests\views\Functional\Update\ViewsFixRevisionIdUpdat
Drupal\Tests\media\Functional\MediaRevisionTest
Drupal\Tests\node\Functional\NodeRevisionsAllTest
Drupal\Tests\node\Functional\NodeRevisionsUiBypassAccessTest
Drupal\Tests\node\Functional\NodeRevisionsTest
Drupal\Tests\node\Functional\NodeRevisionsUiTest
Drupal\Tests\node\Functional\Views\Wizard\NodeRevisionWizard
Drupal\Tests\node\Functional\Views\RevisionLinkTest
Drupal\Tests\views\Functional\Wizard\EntityTestRevisionTes

yes there is existing test coverage of the latest_revision filter

Drupal\Tests\views\Kernel\Entity\LatestRevisionFilterTest

Committed/pushed to 11.x, thanks!

This could theoretically break some hook_query_alter() implementations, so not backporting to 10.1.x. 10.2.x will be out in a couple of months and include this fix.

Thanks @catch

I did a quick grep through the entire contrib space

Only 15 handful of contrib modules using hook_query_alter() and likely very few if any related to the latest_revision Views Filter. Low risk high benefit change for sure.

bene_event
clockify_report
cloudwords
commerce_cart
communities_view
community_builder
dow_jones_search_clone
entity_query_alter
fullcalendar
ga_reports
google_appliance
image_export_import
local_translation
multiversion
nbox

I'm guessing a very small chance of impact to contrib but possibly custom projects , yet again, very small percentage if any. Way more benefits than risk here.

After upgrading core from 10.1.5 to 10.2.2 this patch stopped working:
https://www.drupal.org/files/issues/2021-07-27/drupal-views_filter_lates...

and tested this one, but this also is not applying for me.
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2022-12-16/drupal-views_filter_lates...

Did I read this correct, that the patch has been rolled into 10.2.x?

@jakegibs617
you do not need this patch, it's already included in 10.2.2