Sql id map plugin's getRowByDestination shouldn't return FALSE

🤯

Might I also suggest something like

diff --git a/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
index b55fda4b43..8459315299 100644
--- a/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
+++ b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
@@ -542,7 +542,9 @@ public function getRowByDestination(array $destination_id_values) {
       $query->condition("map.$destination_id", $destination_id_values[$field_name], '=');
     }
     $result = $query->execute();
-    return $result->fetchAssoc();
+    $return = $result->fetchAssoc();
+    assert(is_array($return));
+    return $return;
   }
 
   /**

to not make any change that could break anything (i.e. adding a return type would be better, but might be seen as an API change), but would have helped pinpoint this ages ago?

#7: I just mean "add that assert for the return value".

+++ b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
@@ -539,10 +539,13 @@ public function getRowByDestination(array $destination_id_values) {
+    return $result ? $result : [];

Because this could theoretically still return NULL for example.

(because its interface declares that it has to return

Aha — all good then! I have learned to not trust the database layer 🤓, but in this case that is then unwarranted!

Sorry for derailing this over a nitpick.

This patch has explicit test coverage. It is updating code to actually do what its docs say. I can't find anything else to nitpick here.

Committed 8dea1fb and pushed to 9.3.x. Thanks!