diff --git a/pkg/sql/colexec/is_null_ops.eg.go b/pkg/sql/colexec/is_null_ops.eg.go index a6b2753bde00..f25311db4629 100644 --- a/pkg/sql/colexec/is_null_ops.eg.go +++ b/pkg/sql/colexec/is_null_ops.eg.go @@ -288,8 +288,10 @@ func (o *isTupleNullSelOp) Next() coldata.Batch { if sel := batch.Selection(); sel != nil { sel = sel[:n] for _, i := range sel { - 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 { @@ -302,8 +304,10 @@ func (o *isTupleNullSelOp) Next() coldata.Batch { batch.SetSelection(true) sel := batch.Selection()[:n] for i := range sel { - 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 { diff --git a/pkg/sql/colexec/is_null_ops_tmpl.go b/pkg/sql/colexec/is_null_ops_tmpl.go index ae91efc2b884..9eae4e53122d 100644 --- a/pkg/sql/colexec/is_null_ops_tmpl.go +++ b/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 { diff --git a/pkg/sql/logictest/testdata/logic_test/tuple b/pkg/sql/logictest/testdata/logic_test/tuple index 30748cea7434..8a9d6c42422c 100644 --- a/pkg/sql/logictest/testdata/logic_test/tuple +++ b/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 diff --git a/pkg/sql/sem/eval/eval_test/BUILD.bazel b/pkg/sql/sem/eval/eval_test/BUILD.bazel index 30d7a4eb21ac..d2ced2f95c90 100644 --- a/pkg/sql/sem/eval/eval_test/BUILD.bazel +++ b/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", diff --git a/pkg/sql/sem/eval/eval_test/eval_test.go b/pkg/sql/sem/eval/eval_test/eval_test.go index 0650f6b16ce0..c165f41babc3 100644 --- a/pkg/sql/sem/eval/eval_test/eval_test.go +++ b/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,7 +188,8 @@ 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++ @@ -195,11 +197,22 @@ func TestEval(t *testing.T) { }}, }}, 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()