Merge #40620

40620: coldata: bugfix to coldata.Copy with NULL and SelOnDest r=jordanlewis a=jordanlewis

Previously, Copy always wiped all of the destination NULLs regardless of
SelOnDest, which if set indiciates that only the selection vector tuples
will be written to on the destination side. Now, when SelOnDest is
true, we don't erroneously overwrite non-selected nulls.

Release note: None

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>

pkg/col/coldata/vec_test.go

@@ -359,6 +359,55 @@ func TestCopyNulls(t *testing.T) {
 	}
 }
 
+func TestCopySelOnDestDoesNotUnsetOldNulls(t *testing.T) {
+	const typ = coltypes.Int64
+
+	// Set up the destination vector. It is all nulls except for a single
+	// non-null at index 0.
+	dst := NewMemColumn(typ, BatchSize)
+	dstInts := dst.Int64()
+	for i := range dstInts {
+		dstInts[i] = 1
+	}
+	dst.Nulls().SetNulls()
+	dst.Nulls().UnsetNull(0)
+
+	// Set up the source vector with two nulls.
+	src := NewMemColumn(typ, BatchSize)
+	srcInts := src.Int64()
+	for i := range srcInts {
+		srcInts[i] = 2
+	}
+	src.Nulls().SetNull(0)
+	src.Nulls().SetNull(3)
+
+	// Using a small selection vector and SelOnDest, perform a copy and verify
+	// that nulls in between the selected tuples weren't unset.
+	copyArgs := CopySliceArgs{
+		SelOnDest: true,
+		SliceArgs: SliceArgs{
+			ColType:     typ,
+			Src:         src,
+			SrcStartIdx: 1,
+			SrcEndIdx:   3,
+			Sel:         []uint16{0, 1, 3},
+		},
+	}
+
+	dst.Copy(copyArgs)
+
+	// 0 was not null in dest and null in source, but it wasn't selected. Not null.
+	require.False(t, dst.Nulls().NullAt(0))
+	// 1 was null in dest and not null in source: it becomes not null.
+	require.False(t, dst.Nulls().NullAt(1))
+	// 2 wasn't included in the selection vector: it stays null.
+	require.True(t, dst.Nulls().NullAt(2))
+	// 3 was null in dest and null in source: it stays null.
+	require.True(t, dst.Nulls().NullAt(3))
+	// 4 wasn't included: it stays null.
+	require.True(t, dst.Nulls().NullAt(4))
+}
+
 func BenchmarkAppend(b *testing.B) {
 	const typ = coltypes.Int64
 

pkg/col/coldata/vec_tmpl.go

@@ -93,6 +93,7 @@ func _COPY_WITH_SEL(
 			} else {
 				v := execgen.UNSAFEGET(fromCol, int(selIdx))
 				// {{if .SelOnDest}}
+				m.nulls.UnsetNull64(uint64(selIdx))
 				execgen.SET(toCol, int(selIdx), v)
 				// {{else}}
 				execgen.SET(toCol, i+int(args.DestIdx), v)
@@ -118,7 +119,14 @@ func _COPY_WITH_SEL(
 // */}}
 
 func (m *memColumn) Copy(args CopySliceArgs) {
-	m.Nulls().UnsetNullRange(args.DestIdx, args.DestIdx+(args.SrcEndIdx-args.SrcStartIdx))
+	if !args.SelOnDest {
+		// We're about to overwrite this entire range, so unset all the nulls.
+		m.Nulls().UnsetNullRange(args.DestIdx, args.DestIdx+(args.SrcEndIdx-args.SrcStartIdx))
+	}
+	// } else {
+	// SelOnDest indicates that we're applying the input selection vector as a lens
+	// into the output vector as well. We'll set the non-nulls by hand below.
+	// }
 
 	switch args.ColType {
 	// {{range .}}