db: fix interaction between no-op optimizations and RangeKeyChanged

Previously, RangeKeyChanged could report a false negative when an iterator hit
a no-op seek optimization. These seek optimizations avoid repositioning the
iterator in cases of repeated, monotonically shifting seek keys. These
optimizations reuse the current iterator state for the next position.

If the previous iterator operation was a no-op SetOptions or SetBounds call and
the last seek operation resulted in RangeKeyChanged()=false the iterator could
falsely preserve the RangeKeyChanged()=false state after the no-op seek
operation. The first seek call to return a range key after a SetOptions or
SetBounds call must return RangeKeyChanged()=true.

Close #1950.
Close #1952.

iterator.go

@@ -1015,6 +1015,7 @@ func (i *Iterator) SeekGEWithLimit(key []byte, limit []byte) IterValidityState {
 		i.rangeKey.prevPosHadRangeKey = i.rangeKey.hasRangeKey && i.Valid()
 	}
 	lastPositioningOp := i.lastPositioningOp
+	requiresReposition := i.requiresReposition
 	// Set it to unknown, since this operation may not succeed, in which case
 	// the SeekGE following this should not make any assumption about iterator
 	// position.
@@ -1046,6 +1047,19 @@ func (i *Iterator) SeekGEWithLimit(key []byte, limit []byte) IterValidityState {
 				// Noop
 				if !invariants.Enabled || !disableSeekOpt(key, uintptr(unsafe.Pointer(i))) || i.forceEnableSeekOpt {
 					i.lastPositioningOp = seekGELastPositioningOp
+
+					// If there's a range key iterator stack, we need to update
+					// whether or not the current current key has changed since
+					// the previous iterator position. This is surfaced in the
+					// public interface through Iterator.RangeKeyChanged(). In
+					// most cases, we're reusing the iterator position so if
+					// there's any range key, it hasn't changed. But if
+					// requiresReposition=true, the previous iterator position
+					// was a no-op SetOptions/SetBounds, and RangeKeyChanged()
+					// must return true if there is a range key at the position.
+					if i.rangeKey != nil {
+						i.rangeKey.updated = i.rangeKey.hasRangeKey && requiresReposition
+					}
 					return i.iterValidityState
 				}
 			}
@@ -1260,6 +1274,7 @@ func (i *Iterator) SeekLTWithLimit(key []byte, limit []byte) IterValidityState {
 		i.rangeKey.prevPosHadRangeKey = i.rangeKey.hasRangeKey && i.Valid()
 	}
 	lastPositioningOp := i.lastPositioningOp
+	requiresReposition := i.requiresReposition
 	// Set it to unknown, since this operation may not succeed, in which case
 	// the SeekLT following this should not make any assumption about iterator
 	// position.
@@ -1292,6 +1307,19 @@ func (i *Iterator) SeekLTWithLimit(key []byte, limit []byte) IterValidityState {
 					(limit == nil || i.cmp(limit, i.key) <= 0)) {
 				if !invariants.Enabled || !disableSeekOpt(key, uintptr(unsafe.Pointer(i))) {
 					i.lastPositioningOp = seekLTLastPositioningOp
+
+					// If there's a range key iterator stack, we need to update
+					// whether or not the current current key has changed since
+					// the previous iterator position. This is surfaced in the
+					// public interface through Iterator.RangeKeyChanged(). In
+					// most cases, we're reusing the iterator position so if
+					// there's any range key, it hasn't changed. But if
+					// requiresReposition=true, the previous iterator position
+					// was a no-op SetOptions/SetBounds, and RangeKeyChanged()
+					// must return true if there is a range key at the position.
+					if i.rangeKey != nil {
+						i.rangeKey.updated = i.rangeKey.hasRangeKey && requiresReposition
+					}
 					return i.iterValidityState
 				}
 			}

testdata/rangekeys

@@ -1547,3 +1547,40 @@ a: (., [a-d) @1=foo UPDATED)
 a: (., [a-"a\x00") @1=foo UPDATED)
 .
 a: (., [a-"a\x00") @1=foo UPDATED)
+
+# Regression test for #1950 — Test a no-op call to SeekGE/SeekLT after a
+# SetBounds/SetOptions noop. The SetBounds/SetOptions noop made the iterator
+# appear to be invalidated, but the internal iterator state was preserved.
+# However, if the previous iterator state had a range key, this range key must
+# be considered changed for the purpose of calculating RangeKeyChanged().
+
+combined-iter lower=a upper=z
+seek-lt z
+set-bounds lower=a upper=z
+seek-lt y
+seek-ge 1
+set-bounds lower=a upper=z
+seek-ge a
+----
+a: (., [a-d) @1=foo UPDATED)
+.
+a: (., [a-d) @1=foo UPDATED)
+a: (., [a-d) @1=foo)
+.
+a: (., [a-d) @1=foo UPDATED)
+
+# Similar to the above regression, test that a no-op correctly returns
+# RangeKeyChanged()=false if there's no intervening SetOptions/SetBounds call.
+
+combined-iter lower=a upper=z
+seek-lt z
+seek-lt y
+set-bounds lower=a upper=z
+seek-ge 1
+seek-ge a
+----
+a: (., [a-d) @1=foo UPDATED)
+a: (., [a-d) @1=foo)
+.
+a: (., [a-d) @1=foo UPDATED)
+a: (., [a-d) @1=foo)