entityQuery reference JOINs should specify target_type

Adding the condition makes sense to me. Let's add some test cases as well.

+++ b/src/Query/Tables.php
@@ -16,15 +16,28 @@ class Tables extends BaseTables {
+      $sql_column_type = str_replace('__target_id', '__target_type', $sql_column);

There might be a better way to do this.

Let's add some tests as well.

I ran into this issue on 1.x today.

Tasks and Notes are custom entities.

    $query = $note_storage->getQuery()
      ->condition('der_field_name.entity:task.task_id', $task->id(), '=')

This query should load all notes that belong to the passed in task. Today, this query returned a note that belonged to a different entity that had the same ID as the task.

I added this to my query to fix it for now, but there should probably be a 1.x patch to address this too. This patch doesn't apply to 1.11 (haven't checked 1.x-dev).

    $query = $note_storage->getQuery()
      ->condition('der_field_name.entity:task.task_id', $task->id(), '=')
      ->condition('der_field_name.target_type', $task->getEntityTypeId(), '=')

Now that its been a year and nobody has stepped up for tests, I think it makes sense to merge the bug fix without tests. Otherwise we are keeping bugs around because we are afraid of them coming back!

I'll write some tests and commit this. :)

The benefit of tests, the last patch only worked for basefields, not for config fields.

That being said, we do need basefield EFQ test in DER.

+++ b/src/Query/Tables.php
@@ -16,15 +16,28 @@ class Tables extends BaseTables {
-    return parent::addNextBaseTable($entity_type, $table, $sql_column, $field_storage);
+
+    $join_condition = "%alias.$entity_type_id_key = $table.$sql_column" . $join_condition_type;
+    return $this->sqlQuery->leftJoin($entity_type->getBaseTable(), NULL, $join_condition);

This is an unfortunate change as we are not calling the parent method anymore.

Also need 1.x patch.

TL;DR

Before

LEFT OUTER JOIN {entity_test_rev} "entity_test_rev" ON [entity_test_rev].[id] = [entity_test__field_test].[field_test_target_id_int]\n

After

LEFT OUTER JOIN {entity_test_rev} "entity_test_rev" ON [entity_test_rev].[id] = [entity_test__field_test].[field_test_target_id_int] AND [entity_test__field_test].[field_test_target_type] = 'entity_test_rev'\n

Now with PHPCS fix.

@chx reviewed it on slack. Committed and pushed to 8.x.2.x. Moving it to 8.x-1.x.

Thanks @jibran and @chx

Any chance we can get a new release soon?

Next tag will be released when 9.2.x is tagged.

Since this patch uses the Drupal 9 square bracket syntax, it is incompatible with Drupal 8.

OMG. Happy 20th Drupal anniversary to @JonBob.

Feel free to attach a D8 compatible patch here. I won't be committed but people can still use it. We can drop D8 support in the next release. I'll update the release notes for the latest release.

RE #18: As we have not removed D8 support form the info file. We have only removed test support. I'll add a BC layer for this in the next release so that it can work with both 8 and 9.

After the next release, we'll drop D8 support as we already have dropped D8 testing support.

Moving back to 2.x and NW as we need a BC patch here.

Thanks Jibran, I think the alternative would be to mark the current release unsupported and instead update the composer.json to indicate that the previous release was the last version with D8 support?

Updating the composer.json would mean a new commit which means a new release so why not add BC and properly drop support. We should also mark the current release unsupported.

D8 compatible patch for the current 2.x version.

#24 works ✨ ... had been a nightmare to find the reason for entity queries being broken when using references.

The patch in #24 indeed works!

This seems to save our a** this time, so thanks!

We believe brackets should be replaced by backtick ` everywhere for compatibility