Config entity query "IS NOT NULL" conditions trigger warnings for non-scalar values

Fix looks good to me, thanks!

I think we can do the same for IS NULL, though, as well. Also would be nice to add a quick test. I see we already have \Drupal\KernelTests\Core\Entity\ConfigEntityQueryTest::testConfigEntityQuery() which does some testing with ->exists() so hopefully it's not too much work.

Thanks the patch looks great, including the added test coverage. I have one minor quibble:

+++ b/core/lib/Drupal/Core/Config/Entity/Query/Condition.php
@@ -154,6 +154,14 @@ protected function matchArray(array $condition, array $data, array $needs_matchi
+      $is_set = isset($value);
+      return $should_be_set === $is_set;

I think the putting $is_set into a variable is a bit unnecessary.

Would be awesome if you could quickly fix that, then I can RTBC.

+++ b/core/lib/Drupal/Core/Config/Entity/Query/Condition.php
@@ -154,6 +154,14 @@ protected function matchArray(array $condition, array $data, array $needs_matchi
+    if (in_array($condition['operator'], ['IS NULL', 'IS NOT NULL'])) {

Also, if you don't mind, it would be nice to add the

$strict =
 TRUE

third parameter here, to ensure this actually does what it should. Otherwise this will return unexpected things when e.g. integer 1 is passed as $condition['operator'].

Here is an updated patch and interdiff which implements #9 and #10

Thanks @Dinesh18!

The patch looks great, but I guess I failed at explaining what I meant in #10.

+++ b/core/lib/Drupal/Core/Config/Entity/Query/Condition.php
@@ -154,6 +154,13 @@ protected function matchArray(array $condition, array $data, array $needs_matchi
+    if (in_array($condition['operator'], ['IS NULL', 'IS NOT NULL'], $strict = TRUE)) {

I put the

$strict =
 

part in my previous comment only to explain what the third parameter is for, it's not actually needed in the code. So where you put

$strict =
 TRUE

, you can just put TRUE, that would be perfect.

Thanks again and sorry for not taking the time to explain this properly earlier.

Here is the updated patch and interdiff which implements #14

That's perfect, thank you!

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

+++ b/core/lib/Drupal/Core/Config/Entity/Query/Condition.php
@@ -154,6 +154,13 @@ protected function matchArray(array $condition, array $data, array $needs_matchi
+    // "IS NULL" and "IS NOT NULL" conditions can also deal with array values,
+    // so we return early for them to avoid problems.
+    if (in_array($condition['operator'], ['IS NULL', 'IS NOT NULL'], TRUE)) {
+      $should_be_set = $condition['operator'] === 'IS NOT NULL';
+      return $should_be_set === isset($value);
+    }
...
       // We always want a case-insensitive match.

I would have just written this as

if ($condition['operator'] === 'IS NULL') {
  return FALSE;
}
elseif ($condition['operator'] === 'IS NOT NULL') { 
  return TRUE;
}

Way less complicated logic and easier to read, and no creating both an array and a scalar we don't need.

#21 is missing two isset()s. Either way would be fine by me, but I actually find the way it went in clearer.