roachtest: costfuzz failed - incorrect results with array concatenation and full join

[mgartner]

Costfuzz discovered a correctness bug here. The two identical SELECT statements below return different results.

CREATE TABLE t1 (
  a  INT
);

CREATE TABLE t2 (
  b INT,
  c TIME[]
);

INSERT INTO t1 (a) VALUES (NULL);

INSERT INTO t2 (c) VALUES (ARRAY['03:23:06.042923']);

SELECT ('09:20:35.19023'::TIME || c)::TIME[] AS col_25551
FROM t1
FULL JOIN t2 ON a = b
ORDER BY a;

SET testing_optimizer_random_seed = 9004529874258270860;

SET testing_optimizer_cost_perturbation = 1.0;

SELECT ('09:20:35.19023'::TIME || c)::TIME[] AS col_25551
FROM t1
FULL JOIN t2 ON a = b
ORDER BY a;

The first one produces:

             col_25551
------------------------------------
  {09:20:35.19023,03:23:06.042923}
  {09:20:35.19023,03:23:06.042923}
(2 rows)

While the second one produces:

             col_25551
------------------------------------
  {09:20:35.19023}
  {09:20:35.19023,03:23:06.042923}
(2 rows)

The second one is the correct answer. The query plans are slightly different, but both look valid to me:

diff --git a/qp1 b/qp2
index 8eba362060..dc2fb51e6e 100644
--- a/qp1
+++ b/qp2
@@ -1,42 +1,41 @@
   distribute
    ├── columns: col_25551:10  [hidden: a:1]
    ├── immutable
    ├── stats: [rows=10000]
-   ├── cost: 5612.94388
+   ├── cost: 4135.70093
    ├── ordering: +1
    ├── distribution: us-east1
    ├── input distribution:
    ├── prune: (1,10)
-   └── sort
-        ├── columns: a:1 col_25551:10
-        ├── immutable
-        ├── stats: [rows=10000]
-        ├── cost: 5612.91388
-        ├── ordering: +1
-        ├── prune: (1,10)
    └── project
         ├── columns: col_25551:10 a:1
         ├── immutable
         ├── stats: [rows=10000]
-             ├── cost: 2539.82625
+        ├── cost: 4135.66162
+        ├── ordering: +1
         ├── prune: (1,10)
-             ├── full-join (hash)
+        ├── sort
+        │    ├── columns: a:1 b:5 c:6
+        │    ├── stats: [rows=10000]
+        │    ├── cost: 3876.3125
+        │    ├── ordering: +1
+        │    └── full-join (hash)
         │         ├── columns: a:1 b:5 c:6
         │         ├── stats: [rows=10000]
-             │    ├── cost: 2339.80625
+        │         ├── cost: 2712.64675
         │         ├── scan t1
         │         │    ├── columns: a:1
         │         │    ├── stats: [rows=1000, distinct(1)=100, null(1)=10]
-             │    │    ├── cost: 1084.62
+        │         │    ├── cost: 1026.49187
         │         │    ├── prune: (1)
         │         │    └── unfiltered-cols: (1-4)
         │         ├── scan t2
         │         │    ├── columns: b:5 c:6
         │         │    ├── stats: [rows=1000, distinct(5)=100, null(5)=10]
-             │    │    ├── cost: 1125.02
+        │         │    ├── cost: 1607.78625
         │         │    ├── prune: (5,6)
         │         │    └── unfiltered-cols: (5-9)
         │         └── filters
         │              └── a:1 = b:5 [outer=(1,5), constraints=(/1: (/NULL - ]; /5: (/NULL - ]), fd=(1)==(5), (5)==(1)]
         └── projections
              └── '09:20:35.19023' || c:6 [as=col_25551:10, outer=(6), immutable]

Jira issue: CRDB-19594

[DrewKimball]

This one is happening because of the null-handling (or lack thereof) for projecting a concatenation between datums in the vectorized engine. Currently, if the projection ignores nulls (all other projections), we check whether the argument is null and simply skip that row if it is. Otherwise, we read the value from the argument column and proceed as normal, even if the value is supposed to be null. My understanding of the way nulls are represented is that if the Nulls slice indicates a value is null, the actual value in the column could be anything and should be ignored. So probably the solution will be to explicitly pass a DNull value in the case when the argument is null and nulls are not ignored.

cockroach/pkg/sql/colexec/colexecprojconst/proj_const_ops_tmpl.go

Lines 165 to 169 in 9b3b3b6

	// {{if and (eq .Left.VecMethod "Datum") (eq .Right.VecMethod "Datum")}}
	hasNullsAndNotCalledOnNullInput := vec.Nulls().MaybeHasNulls() && !p.calledOnNullInput
	// {{else}}
	hasNullsAndNotCalledOnNullInput := vec.Nulls().MaybeHasNulls()
	// {{end}}