feat!: Iterator Key() and Value() no longer return a copy

backend_test.go

@@ -348,7 +348,8 @@ func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) {
 
 	var list []int64
 	for itr.Valid() {
-		key := itr.Key()
+		key := make([]byte, len(itr.Key()))
+		copy(key, itr.Key())
 		list = append(list, bytes2Int64(key))
 		itr.Next()
 	}
@@ -450,7 +451,10 @@ func assertKeyValues(t *testing.T, db DB, expect map[string][]byte) {
 	actual := make(map[string][]byte)
 	for ; iter.Valid(); iter.Next() {
 		require.NoError(t, iter.Error())
-		actual[string(iter.Key())] = iter.Value()
+
+		value := make([]byte, len(iter.Value()))
+		copy(value, iter.Value())
+		actual[string(iter.Key())] = value
 	}
 
 	assert.Equal(t, expect, actual)

badger_db.go

@@ -279,19 +279,22 @@ func (i *badgerDBIterator) Valid() bool {
 	return true
 }
 
+// Key implements Iterator.
+// The caller should not modify the contents of the returned slice.
+// Instead, the caller should make a copy and work on the copy.
 func (i *badgerDBIterator) Key() []byte {
 	if !i.Valid() {
 		panic("iterator is invalid")
 	}
-	// Note that we don't use KeyCopy, so this is only valid until the next
-	// call to Next.
-	return i.iter.Item().KeyCopy(nil)
+	return i.iter.Item().Key()
 }
 
+// Value implements Iterator.
 func (i *badgerDBIterator) Value() []byte {
 	if !i.Valid() {
 		panic("iterator is invalid")
 	}
+
 	val, err := i.iter.Item().ValueCopy(nil)
 	if err != nil {
 		i.lastErr = err

common_test.go

@@ -72,7 +72,9 @@ func checkKeyPanics(t *testing.T, itr Iterator) {
 
 func checkValuePanics(t *testing.T, itr Iterator) {
 	t.Helper()
-	assert.Panics(t, func() { itr.Value() })
+
+	msg := "checkValuePanics expected panic but didn't"
+	assert.Panics(t, func() { itr.Value() }, msg)
 }
 
 func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {

goleveldb_iterator.go

@@ -93,19 +93,19 @@ func (itr *goLevelDBIterator) Valid() bool {
 }
 
 // Key implements Iterator.
+// The caller should not modify the contents of the returned slice.
+// Instead, the caller should make a copy and work on the copy.
 func (itr *goLevelDBIterator) Key() []byte {
-	// Key returns a copy of the current key.
-	// See https://github.com/syndtr/goleveldb/blob/52c212e6c196a1404ea59592d3f1c227c9f034b2/leveldb/iterator/iter.go#L88
 	itr.assertIsValid()
-	return cp(itr.source.Key())
+	return itr.source.Key()
 }
 
 // Value implements Iterator.
+// The caller should not modify the contents of the returned slice.
+// Instead, the caller should make a copy and work on the copy.
 func (itr *goLevelDBIterator) Value() []byte {
-	// Value returns a copy of the current value.
-	// See https://github.com/syndtr/goleveldb/blob/52c212e6c196a1404ea59592d3f1c227c9f034b2/leveldb/iterator/iter.go#L88
 	itr.assertIsValid()
-	return cp(itr.source.Value())
+	return itr.source.Value()
 }
 
 // Next implements Iterator.

pebble.go

@@ -396,19 +396,19 @@ func (itr *pebbleDBIterator) Valid() bool {
 }
 
 // Key implements Iterator.
+// The caller should not modify the contents of the returned slice.
+// Instead, the caller should make a copy and work on the copy.
 func (itr *pebbleDBIterator) Key() []byte {
-	// Key returns a copy of the current key.
-	// See https://github.com/cockroachdb/pebble/blob/v1.0.0/iterator.go#L2106
 	itr.assertIsValid()
-	return cp(itr.source.Key())
+	return itr.source.Key()
 }
 
 // Value implements Iterator.
+// The caller should not modify the contents of the returned slice.
+// Instead, the caller should make a copy and work on the copy.
 func (itr *pebbleDBIterator) Value() []byte {
-	// Value returns a copy of the current value.
-	// See https://github.com/cockroachdb/pebble/blob/v1.0.0/iterator.go#L2116
 	itr.assertIsValid()
-	return cp(itr.source.Value())
+	return itr.source.Value()
 }
 
 // Next implements Iterator.

types.go

@@ -136,11 +136,17 @@ type Iterator interface {
 	Next()
 
 	// Key returns the key at the current position. Panics if the iterator is invalid.
-	// CONTRACT: key readonly []byte
+	// Key returns the key of the current key/value pair, or nil if done.
+	// The caller should not modify the contents of the returned slice, and
+	// its contents may change on the next call to any 'seeks method'.
+	// Instead, the caller should make a copy and work on the copy.
 	Key() (key []byte)
 
 	// Value returns the value at the current position. Panics if the iterator is invalid.
-	// CONTRACT: value readonly []byte
+	// Value returns the value of the current key/value pair, or nil if done.
+	// The caller should not modify the contents of the returned slice, and
+	// its contents may change on the next call to any 'seeks method'.
+	// Instead, the caller should make a copy and work on the copy.
 	Value() (value []byte)
 
 	// Error returns the last error encountered by the iterator, if any.