fix: logical null handling (#5192)

* fix: treat null inputs to `vectorAdd`, `vectorOr` as false

> N.b. this approach was abandoned in favor of a more comprehensive
> change which also addresses some deviation to the flux lang spec
> regarding null handling in logical expressions.
> The comprehensive fix comes in the revs that follow.

Formerly, when the input to vectorized logical ops was a null, we'd see:

```
runtime error: cannot use vectorized or operation on non-vector invalid
```

The precedent set by the row-based logical ops is to treat incoming
nulls as false.

This diff extracts the input validation to a separate function which is
responsible for transitioning the incoming `values.Value` to a `values.Vector`
giving it the chance to intercept the problem null case and convert them
to a new vec repeat holding a boolean false.

* feat: add logicalStrictNullEvaluator

A new version of the logical evaluator which matches the spec.
The first thing it does, is check for a feature flag and will delegate
to the original version when not enabled.

This is not ideal. It would be better to branch higher up in the
compiler, but we don't have easy access to the context to read the
flagger at that level.

BREAKING CHANGE: nulls propagate with the `strictNullLogicalOps` flag on

* fix: null handling for vectorOr vectorAnd WRT elements

* refactor: fixup row-based to add short circuiting back

As I realized these logical ops are commutative I initially thought it
meant short circuiting wouldn't be possible (since we'd need to know the
null positions on both sides) but this was not true.

We can skip evaluating the right side of the op when we see the terminal
cases of a `true` for OR, and `false` for AND. Nulls still need to be
considered carefully case by case when we resort to looking at the right-hand
side, but we can still short circuit.

* refactor: vectorized And/Or with short-circuiting and strict null handling

* refactor: add flagged impl for strict null handling in interp logicals

* test: add testcases for the spec's contract for logicals and nulls

* test: add Go-side tests for vectorized logical exprs

Tried to exercise the short-circuit and null handling paths to check for
memory accounting defects.

* refactor: extract vec logical short-circuit attempts into helper func

* chore: mention how untyped nulls are introduced in tests

* test: add tests for vec repeats in logicals WRT nulls

* chore: make generate

array/binary.go

@@ -15,42 +15,60 @@ func And(l, r *Boolean, mem memory.Allocator) (*Boolean, error) {
 	b := NewBooleanBuilder(mem)
 	b.Resize(n)
 	for i := 0; i < n; i++ {
-		if l.IsValid(i) && r.IsValid(i) {
-			b.Append(l.Value(i) && r.Value(i))
-		} else {
-			b.AppendNull()
+		var elmL *bool
+		if l.IsValid(i) {
+			x := l.Value(i)
+			elmL = &x
 		}
-	}
-	a := b.NewBooleanArray()
-	b.Release()
-	return a, nil
-}
-
-func AndLConst(l bool, r *Boolean, mem memory.Allocator) (*Boolean, error) {
-	n := r.Len()
-	b := NewBooleanBuilder(mem)
-	b.Resize(n)
-	for i := 0; i < n; i++ {
+		var elmR *bool
 		if r.IsValid(i) {
-			b.Append(l && r.Value(i))
-		} else {
+			x := r.Value(i)
+			elmR = &x
+		}
+
+		if elmL == nil && elmR == nil {
+			// both sides are null
 			b.AppendNull()
+		} else if elmL == nil || elmR == nil {
+			// one side is null, the other is false
+			if (elmL != nil && !*elmL) || (elmR != nil && !*elmR) {
+				b.Append(false)
+			} else {
+				b.AppendNull()
+			}
+		} else {
+			// no nulls
+			b.Append(*elmL && *elmR)
 		}
 	}
 	a := b.NewBooleanArray()
 	b.Release()
 	return a, nil
 }
 
-func AndRConst(l *Boolean, r bool, mem memory.Allocator) (*Boolean, error) {
-	n := l.Len()
+func AndConst(fixed *bool, arr *Boolean, mem memory.Allocator) (*Boolean, error) {
+	n := arr.Len()
 	b := NewBooleanBuilder(mem)
 	b.Resize(n)
 	for i := 0; i < n; i++ {
-		if l.IsValid(i) {
-			b.Append(l.Value(i) && r)
-		} else {
+		var elm *bool
+		if arr.IsValid(i) {
+			x := arr.Value(i)
+			elm = &x
+		}
+		if fixed == nil && elm == nil {
+			// both sides are null
 			b.AppendNull()
+		} else if fixed == nil || elm == nil {
+			// one side is null, the other is false
+			if (fixed != nil && !*fixed) || (elm != nil && !*elm) {
+				b.Append(false)
+			} else {
+				b.AppendNull()
+			}
+		} else {
+			// no nulls
+			b.Append(*fixed && *elm)
 		}
 	}
 	a := b.NewBooleanArray()
@@ -67,42 +85,60 @@ func Or(l, r *Boolean, mem memory.Allocator) (*Boolean, error) {
 	b := NewBooleanBuilder(mem)
 	b.Resize(n)
 	for i := 0; i < n; i++ {
-		if l.IsValid(i) && r.IsValid(i) {
-			b.Append(l.Value(i) || r.Value(i))
-		} else {
-			b.AppendNull()
+		var elmL *bool
+		if l.IsValid(i) {
+			x := l.Value(i)
+			elmL = &x
 		}
-	}
-	a := b.NewBooleanArray()
-	b.Release()
-	return a, nil
-}
-
-func OrLConst(l bool, r *Boolean, mem memory.Allocator) (*Boolean, error) {
-	n := r.Len()
-	b := NewBooleanBuilder(mem)
-	b.Resize(n)
-	for i := 0; i < n; i++ {
+		var elmR *bool
 		if r.IsValid(i) {
-			b.Append(l || r.Value(i))
-		} else {
+			x := r.Value(i)
+			elmR = &x
+		}
+
+		if elmL == nil && elmR == nil {
+			// both sides are null
 			b.AppendNull()
+		} else if elmL == nil || elmR == nil {
+			// one side is null, the other is true
+			if (elmL != nil && *elmL) || (elmR != nil && *elmR) {
+				b.Append(true)
+			} else {
+				b.AppendNull()
+			}
+		} else {
+			// no nulls
+			b.Append(*elmL || *elmR)
 		}
 	}
 	a := b.NewBooleanArray()
 	b.Release()
 	return a, nil
 }
 
-func OrRConst(l *Boolean, r bool, mem memory.Allocator) (*Boolean, error) {
-	n := l.Len()
+func OrConst(fixed *bool, arr *Boolean, mem memory.Allocator) (*Boolean, error) {
+	n := arr.Len()
 	b := NewBooleanBuilder(mem)
 	b.Resize(n)
 	for i := 0; i < n; i++ {
-		if l.IsValid(i) {
-			b.Append(l.Value(i) || r)
-		} else {
+		var elm *bool
+		if arr.IsValid(i) {
+			x := arr.Value(i)
+			elm = &x
+		}
+		if fixed == nil && elm == nil {
+			// both sides are null
 			b.AppendNull()
+		} else if fixed == nil || elm == nil {
+			// one side is null, the other is true
+			if (fixed != nil && *fixed) || (elm != nil && *elm) {
+				b.Append(true)
+			} else {
+				b.AppendNull()
+			}
+		} else {
+			// no nulls
+			b.Append(*fixed || *elm)
 		}
 	}
 	a := b.NewBooleanArray()

compiler/compiler.go

@@ -634,8 +634,11 @@ func compile(n semantic.Node, subst semantic.Substitutor) (Evaluator, error) {
 				right:    r,
 			}, nil
 		}
-
-		return &logicalEvaluator{
+		// TODO(onelson): try to select strict or regular logical here.
+		//   Need ctx to read flagger.
+		//   Currently this happens during Eval() where ctx is available and
+		//   will fallback to the regular logicalEvaluator if the flag is unset.
+		return &logicalStrictNullEvaluator{
 			operator: n.Operator,
 			left:     l,
 			right:    r,