Merge #101800

101800: copy: fix copy partial index handling r=cucaroach a=cucaroach

Copy partial index handling in the new vector encoder was broken and
failing but we didn't know it because the copy-from-kvtrace test
command was ignoring errors (the trace was still happening but we
ignored the "empty kv" error from KV).

Now copy-from-kvtrace errors if the copy errors and in order to fix
the underlying problem kvSparseSliceBulkIterator ACTUALLY supports
sparse slices meaning it skips over empty keys.

We went back and forth on where/who should do this sparse skipping and
somehow it got dropped and the tests designed to flex didn't catch it
because of the above error.

In the real world the ramifications of this would be that COPY's into
tables with partial indexes would just always fail if any rows were
filtered out. This could be worked around in betas by disabling the
vectorized implementation of COPY.

The only reason we caught this is due to metamorphic testing that would
sometimes send the 3 rows in two batches and the kvtrace the test
would see wouldn't have the 3rd row because the inserter bailed on the
error.  Yeah for metamorphic testing!

Epic: none
Release note: none
Fixes: #101609


Co-authored-by: Tommy Reilly <[email protected]>

pkg/sql/copy/copy_test.go

@@ -143,6 +143,7 @@ func TestDataDriven(t *testing.T) {
 								require.Error(t, err, "copy-from-error didn't return and error!")
 								return err.Error()
 							case "copy-from-kvtrace":
+								require.NoError(t, err, "%s\n%s\n", d.Cmd, d.Input)
 								rows, err := conn.Query(ctx,
 									`SELECT
 						  regexp_replace(message, '/Table/[0-9]*/', '/Table/<>/')

pkg/sql/copy/testdata/copy_from

@@ -602,13 +602,13 @@ COPY tenum FROM STDIN
 
 copy-from-kvtrace
 COPY tenum FROM STDIN
-0	cat
-1	dog
-2	bear
+3	cat
+4	dog
+5	bear
 ----
-CPut /Table/<>/1/0/0 -> /TUPLE/2:2:Bytes/@
-CPut /Table/<>/1/1/0 -> /TUPLE/2:2:Bytes/0x80
-CPut /Table/<>/1/2/0 -> /TUPLE/2:2:Bytes/0xc0
+CPut /Table/<>/1/3/0 -> /TUPLE/2:2:Bytes/@
+CPut /Table/<>/1/4/0 -> /TUPLE/2:2:Bytes/0x80
+CPut /Table/<>/1/5/0 -> /TUPLE/2:2:Bytes/0xc0
 
 exec-ddl
 CREATE TABLE tenum2 (i INT PRIMARY KEY, c1 testenum, INDEX(c1))
@@ -622,10 +622,10 @@ COPY tenum2 FROM STDIN
 
 copy-from-kvtrace
 COPY tenum2 FROM STDIN
-0	cat
+1	cat
 ----
-CPut /Table/<>/1/0/0 -> /TUPLE/2:2:Bytes/@
-InitPut /Table/<>/2/"@"/0/0 -> /BYTES/
+CPut /Table/<>/1/1/0 -> /TUPLE/2:2:Bytes/@
+InitPut /Table/<>/2/"@"/1/0 -> /BYTES/
 
 exec-ddl
 CREATE TABLE tenum3 (i INT PRIMARY KEY, c1 testenum, UNIQUE INDEX(c1))
@@ -639,10 +639,10 @@ COPY tenum3 FROM STDIN
 
 copy-from-kvtrace
 COPY tenum3 FROM STDIN
-0	cat
+1	dog
 ----
-CPut /Table/<>/1/0/0 -> /TUPLE/2:2:Bytes/@
-InitPut /Table/<>/2/"@"/0 -> /BYTES/0x88
+CPut /Table/<>/1/1/0 -> /TUPLE/2:2:Bytes/0x80
+InitPut /Table/<>/2/"\x80"/0 -> /BYTES/0x89
 
 exec-ddl
 CREATE TYPE comp AS (a INT, b INT);
@@ -657,6 +657,6 @@ COPY tcomp FROM STDIN
 
 copy-from-kvtrace
 COPY tcomp FROM STDIN
-0	(1, 2)
+1	(1, 2)
 ----
-CPut /Table/<>/1/0/0 -> /TUPLE/
+CPut /Table/<>/1/1/0 -> /TUPLE/

pkg/sql/row/putter.go

@@ -238,7 +238,13 @@ type kvSparseSliceBulkSource[T kv.GValue] struct {
 var _ kv.BulkSource[[]byte] = &kvSparseSliceBulkSource[[]byte]{}
 
 func (k *kvSparseSliceBulkSource[T]) Len() int {
-	return len(k.keys)
+	cnt := 0
+	for _, k := range k.keys {
+		if len(k) > 0 {
+			cnt++
+		}
+	}
+	return cnt
 }
 
 type sliceIterator[T kv.GValue] struct {
@@ -253,9 +259,13 @@ func (k *kvSparseSliceBulkSource[T]) Iter() kv.BulkSourceIterator[T] {
 }
 
 func (s *sliceIterator[T]) Next() (roachpb.Key, T) {
-	k, v := s.s.keys[s.cursor], s.s.values[s.cursor]
-	s.cursor++
-	return k, v
+	for {
+		k, v := s.s.keys[s.cursor], s.s.values[s.cursor]
+		s.cursor++
+		if len(k) > 0 {
+			return k, v
+		}
+	}
 }
 
 // KVBatchAdapter implements Putter interface and adapts it to kv.Batch API.