fix(y): shall always return empty slice rather than nil (#2245)

Fixes #2067

**Description**

This PR fixes an issue where Go's `append` returns `nil` when appending
an empty slice to a nil slice.
I added a check in `SafeCopy` to ensure we always return `[]byte{}` in
this case, along with a new unit test.
I also verified it using the test offered in the issue

**Checklist**

- [x] Code compiles correctly and linting passes locally
- [ ] For all _code_ changes, an entry added to the `CHANGELOG.md` file
describing and linking to
      this PR
- [x] Tests added for new functionality, or regression tests for bug
fixes added as applicable

---------

Co-authored-by: Matthew McNeely <matthew.mcneely@gmail.com>

stream_writer_test.go

@@ -754,16 +754,16 @@ func TestStreamWriterIncremental(t *testing.T) {
 			require.NoError(t, sw.Write(buf), "sw.Write() failed")
 			require.NoError(t, sw.Flush(), "sw.Flush() failed")
 
-			buf = z.NewBuffer(10<<20, "test")
-			defer func() { require.NoError(t, buf.Release()) }()
+			buf2 := z.NewBuffer(10<<20, "test")
+			defer func() { require.NoError(t, buf2.Release()) }()
 			KVToBuffer(&pb.KV{
 				Key:     []byte("a2"),
 				Value:   []byte("val2"),
 				Version: 9,
-			}, buf)
+			}, buf2)
 			sw = db.NewStreamWriter()
 			require.NoError(t, sw.PrepareIncremental(), "sw.PrepareIncremental() failed")
-			require.NoError(t, sw.Write(buf), "sw.Write() failed")
+			require.NoError(t, sw.Write(buf2), "sw.Write() failed")
 			require.NoError(t, sw.Flush(), "sw.Flush() failed")
 
 			// This will move the maxTs to 10 (earlier, without the fix)

test.sh

@@ -106,6 +106,6 @@ write_coverage() {
 # parallel tests currently not working
 # parallel --halt now,fail=1 --progress --line-buffer ::: stream manual root
 # run tests in sequence
-#root
-#stream
+root
+stream
 manual

y/y.go

@@ -95,7 +95,11 @@ func OpenTruncFile(filename string, sync bool) (*os.File, error) {
 
 // SafeCopy does append(a[:0], src...).
 func SafeCopy(a, src []byte) []byte {
-	return append(a[:0], src...)
+	b := append(a[:0], src...)
+	if b == nil {
+		return []byte{}
+	}
+	return b
 }
 
 // Copy copies a byte slice and returns the copied slice.
@@ -134,7 +138,7 @@ func CompareKeys(key1, key2 []byte) int {
 
 // ParseKey parses the actual key from the key bytes.
 func ParseKey(key []byte) []byte {
-	if key == nil {
+	if len(key) < 8 {
 		return nil
 	}
 

y/y_test.go

@@ -328,3 +328,47 @@ func TestAllocatorReuse(t *testing.T) {
 	}
 	t.Logf("Allocator: %s\n", a)
 }
+
+func TestSafeCopy_Issue2067(t *testing.T) {
+	type args struct {
+		a   []byte
+		src []byte
+	}
+	tests := []struct {
+		name string
+		args args
+		want []byte
+	}{
+		{
+			name: "Nil src should return empty slice not nil",
+			args: args{a: nil, src: nil},
+			want: []byte{},
+		},
+		{
+			name: "Empty src should return empty slice not nil",
+			args: args{a: nil, src: []byte{}},
+			want: []byte{},
+		},
+		{
+			name: "Normal src should return src content",
+			args: args{a: nil, src: []byte("hello")},
+			want: []byte("hello"),
+		},
+		{
+			name: "Buffer reuse with nil src should return empty slice",
+			args: args{a: make([]byte, 10), src: nil},
+			want: []byte{},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := SafeCopy(tt.args.a, tt.args.src)
+			require.Equal(t, tt.want, got)
+
+			// Explicit check for nil vs empty slice distinction
+			if len(tt.want) == 0 {
+				require.NotNil(t, got, "SafeCopy returned nil, but we expected an empty slice []byte{}")
+			}
+		})
+	}
+}