Views which load the same entity type as entity and non default revision cause fatal in Sql::getCacheTags()

Fixed the fail patch as well.

Some minor feedback other that this is RTBC.

1. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
   @@ -1487,58 +1487,73 @@ public function loadEntities(&$results) {
   +          // Drupal core currently has no way to load multiple revisions. Sad.
   

   We can be little optimistic in comment.

+++ b/core/modules/views/tests/src/Kernel/Plugin/SqlEntityLoadingTest.php
@@ -0,0 +1,82 @@
+ * @see \Drupal\views\Plugin\views\query\Sql
+ * @group views

Technically @see should be at the end of the doc block.

I tested the patch in #14, and it worked for me and fixed #2714717: Content (nid) relationship joins to wrong nodes. I'm probably not deep enough into this to RTBC it, though.

Thanks @dawehner for fixing feedback.

+++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -1487,58 +1487,73 @@ public function loadEntities(&$results) {
+
+  protected function assignEntitiesToResult($ids, array $entities, array $results) {

Missing doc block.

I guess we can backport this to 8.1.x by adding an _ to assignEntitiesToResult() if we want.

Reviewing the code here is painful.

Seems perfect now.

Committed e888be3 and pushed to 8.2.x. Thanks!

@dawehner nice coverage. Using the _ method we can port this fix to 8.1.x

diff --git a/core/modules/views/src/Plugin/views/query/Sql.php b/core/modules/views/src/Plugin/views/query/Sql.php
index 6f8ab15..cfe593c 100644
--- a/core/modules/views/src/Plugin/views/query/Sql.php
+++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -5,7 +5,6 @@
 use Drupal\Component\Utility\NestedArray;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Database\Database;
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\views\Plugin\views\display\DisplayPluginBase;

Unused use removed on commit.

Seems like a novice task.

It's green and it prefixes a new protected method with an underscore as mentioned in #2744157: [policy, no patch] Clarify the policy around backporting changes with potential issues. It changes nothing aside from that, so looks like a clean backport to me.

Committed c6ed7c9 and pushed to 8.1.x. Thanks!

I was having the same symptoms with a custom view in Drupal 8.3.7. I went through all the relations and marked them "required". This fixed the issue. Just noting this here for findability.

Having this problem on 8.4.0. The view is using the content_revisions base table and has a relationship to both the moderation_state as well as the main node. Requiring the relationships did not resolve the issue as in the previous comment.

When deleting a revision, returning to this revisions view afterwards triggered the error.

I completely disabled caching on that display and the issue went away.

Having the same issue on 8.4.0. I tried disabling caching (caching: none in advanced view settings) as mentioned in #34, but still issue happens.