Equality comparison quirk

[sarathkcm]

The behaviour here is quite surprising.

I would expect equality-for-non-existent-field to be false and non-equality-for-non-existent-field to be true.

Is it a bug or expected behaviour?

[andrew-coleman]

Hmm, this does seem odd. Most operators will return the empty sequence (i.e. nothing / JS undefined) if either operand evaluates to empty. However, these both return false. I'll give this some thought.

[sarathkcm]

It does make sense that mathematical (arithmetic + comparison) operators will return undefined if any operand is empty. Since equality and non-equality operators apply to types beyond numbers, in my opinion, it should return true or false similar to how JS would have.

[johanventer]

I just came across this trying to explain to someone why this happens (where there is no input, so x is undefined):

x = "hello" // false
x != "hello" // false

The cause is obviously this:

    function evaluateEqualityExpression(lhs, rhs, op) {
        var result;

        // type checks
        var ltype = typeof lhs;
        var rtype = typeof rhs;

        if (ltype === 'undefined' || rtype === 'undefined') {
            // if either side is undefined, the result is false
            return false;
        }

This is probably fine behaviour for =, but it's completely unintuitive for !=:

<undefined> = <any>  // false, cool
<undefined> != <any> // false, not cool

The inequality operator probably needs to be special cased here. I imagine this would be a major breaking change at this point, but the current behaviour is a massive foot gun.

[johanventer]

I hit this again today with someone coming to ask me why an expression doesn't work:

undefined                   /* undefined */
$not(undefined)             /* undefined */

$boolean(undefined)         /* undefined */
$not($boolean(undefined))   /* undefined */

Again the issue with changing this behavior is that it's backwards-incompatible, but would make things so much more sane.

I took a quick look at this and it seems that changing the way the equality comparisons work is maybe too much, as you would end up with different results from sequence filtering. However, just having $boolean return false for an undefined value would at least make this work and doesn't break any of the other tests:

$not(undefined)                   /* true */
$boolean(undefined)               /* false */
$not($boolean(undefined))         /* true*/

Here's a patch, but I'm not going to make it a PR as I'm not sure if there are other consequences:

--- a/src/functions.js
+++ b/src/functions.js
@@ -1390,15 +1390,11 @@ const functions = (() => {
         // string: zero-length -> false; otherwise -> true
         // number: 0 -> false; otherwise -> true
         // null -> false
+        // undefined -> false
         // array: empty -> false; length > 1 -> true
         // object: empty -> false; non-empty -> true
         // function -> false

-        // undefined inputs always return undefined
-        if (typeof arg === 'undefined') {
-            return undefined;
-        }
-
         var result = false;
         if (Array.isArray(arg)) {
             if (arg.length === 1) {
diff --git a/test/test-suite/groups/function-boolean/case020.json b/test/test-suite/groups/function-boolean/case020.json
index 62f79c3..e088259 100644
--- a/test/test-suite/groups/function-boolean/case020.json
+++ b/test/test-suite/groups/function-boolean/case020.json
@@ -2,5 +2,5 @@
     "expr": "$boolean(Account.blah)",
     "dataset": "dataset5",
     "bindings": {},
-    "undefinedResult": true
+    "result": false
 }