Merge pull request #126938 from cockroachdb/blathers/backport-release…

…-24.1-126901

release-24.1: colexec: fix IS NOT NULL filter for tuples with NULLs

pkg/sql/colexec/is_null_ops_tmpl.go

@@ -244,8 +244,10 @@ func _MAYBE_SELECT(
 	// {{define "maybeSelect" -}}
 
 	// {{if .IsTuple}}
-	selectTuple := nulls.NullAt(i) != o.negate
-	if !selectTuple {
+	var selectTuple bool
+	if nulls.NullAt(i) {
+		selectTuple = !o.negate
+	} else {
 		selectTuple = isTupleNull(datums.Get(i).(tree.Datum), o.negate)
 	}
 	if selectTuple {

pkg/sql/logictest/testdata/logic_test/tuple

@@ -1307,3 +1307,28 @@ RESET testing_optimizer_random_seed;
 RESET testing_optimizer_disable_rule_probability;
 RESET vectorize;
 RESET distsql_workmem;
+
+# Regression test for incorrect handling of IS NOT NULL filter in the vectorized
+# engine (#126769).
+query T
+SELECT * FROM (SELECT (NULL, NULL) AS a) AS b WHERE a IS NOT NULL;
+----
+
+query T
+SELECT * FROM (SELECT (1, NULL) AS a) AS b WHERE a IS NOT NULL;
+----
+
+query T
+SELECT * FROM (SELECT (1, 2) AS a) AS b WHERE a IS NOT NULL;
+----
+(1,2)
+
+query B
+SELECT ROW() IS NULL;
+----
+true
+
+query B
+SELECT ROW() IS NOT NULL;
+----
+true

pkg/sql/sem/eval/eval_test/BUILD.bazel

@@ -23,7 +23,6 @@ go_test(
         "//pkg/sql/execinfra",
         "//pkg/sql/execinfrapb",
         "//pkg/sql/parser",
-        "//pkg/sql/rowenc",
         "//pkg/sql/rowexec",
         "//pkg/sql/sem/builtins",
         "//pkg/sql/sem/eval",

pkg/sql/sem/eval/eval_test/eval_test.go

@@ -29,7 +29,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
-	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
 	_ "github.com/cockroachdb/cockroach/pkg/sql/sem/builtins"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
@@ -38,6 +37,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/datadriven"
 	"github.com/stretchr/testify/require"
 )
@@ -141,6 +141,7 @@ func TestEval(t *testing.T) {
 	})
 
 	t.Run("vectorized", func(t *testing.T) {
+		rng, _ := randutil.NewTestRand()
 		var monitorRegistry colexecargs.MonitorRegistry
 		defer monitorRegistry.Close(ctx)
 		walk(t, func(t *testing.T, d *datadriven.TestData) string {
@@ -187,19 +188,31 @@ func TestEval(t *testing.T) {
 							if batchesReturned > 0 {
 								return coldata.ZeroBatch
 							}
-							// It doesn't matter what types we create the input batch with.
+							// It doesn't matter what types we create the input
+							// batch with.
 							batch := coldata.NewMemBatch([]*types.T{}, coldata.StandardColumnFactory)
 							batch.SetLength(1)
 							batchesReturned++
 							return batch
 						}},
 				}},
 				StreamingMemAccount: &acc,
-				// Unsupported post processing specs are wrapped and run through the
-				// row execution engine.
+				// Unsupported post-processing specs are wrapped and run through
+				// the row execution engine.
 				ProcessorConstructor: rowexec.NewProcessor,
 				MonitorRegistry:      &monitorRegistry,
 			}
+			// If the expression is of the boolean type, in 50% cases we'll
+			// additionally run it as a filter (i.e. as a "selection" operator
+			// as opposed to a "projection").
+			var doFilter bool
+			if typedExpr.ResolvedType().Family() == types.BoolFamily && rng.Float64() < 0.5 {
+				doFilter = true
+				args.Spec.Core.Noop = nil
+				args.Spec.Core.Filterer = &execinfrapb.FiltererSpec{
+					Filter: execinfrapb.Expression{LocalExpr: typedExpr},
+				}
+			}
 			result, err := colbuilder.NewColOperator(ctx, flowCtx, args)
 			require.NoError(t, err)
 
@@ -211,19 +224,31 @@ func TestEval(t *testing.T) {
 				[]*types.T{typedExpr.ResolvedType()},
 			)
 
-			var (
-				row  rowenc.EncDatumRow
-				meta *execinfrapb.ProducerMetadata
-			)
 			mat.Start(ctx)
-			row, meta = mat.Next()
+			row, meta := mat.Next()
 			if meta != nil {
 				if meta.Err != nil {
 					return fmt.Sprint(meta.Err)
 				}
 				t.Fatalf("unexpected metadata: %+v", meta)
 			}
-			if row == nil {
+			if doFilter {
+				switch strings.ToLower(strings.TrimSpace(d.Expected)) {
+				case "true":
+					if row == nil {
+						t.Fatalf("the row should not be filtered out: %s", d.Input)
+					}
+				case "false", "null":
+					if row != nil {
+						t.Fatalf("the row should be filtered out: %s", d.Input)
+					}
+					// The row is filtered out, so we just return the expected
+					// output here.
+					return strings.TrimSpace(d.Expected)
+				default:
+					t.Fatalf("unexpected bool value: %s", d.Expected)
+				}
+			} else if row == nil {
 				t.Fatal("unexpected end of input")
 			}
 			return row[0].Datum.String()