db: fix iterator error propagation, NextPrefix violation

[jbowens]

This commit fixes a few issues surrounding NextPrefix and iterator errors. Previously, under some circumstances, an error propagated by the underlying iterator would not immediately be propagated up. Instead the Iterator could in some circumstances call Next or NextPrefix on an already exhausted, errored iterator, resulting in undefined behavior.

Similarly, the results of calling NextPrefix on an exhausted top-level pebble.Iterator are not defined. This commit adapts the implementation to early exit in this case. The metamorphic test may generate such operations.

Fix #3592.

[cockroach-teamcity]

This change is 

[RaduBerinde]

Just one thing. this does not fix the original violates upper bound issue in #3592.

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @aadityasondhi)

[jbowens]

Pushed a second commit to fix the violates upper bound issue 😅

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @aadityasondhi)

[RaduBerinde]

Nice!!

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @aadityasondhi and @jbowens)

---

internal/keyspan/keyspanimpl/level_iter.go line 158 at r2 (raw file):

		return noFileLoaded
	}
	l.iter, l.err = l.newIter(file, l.tableOpts)

[nit] maybe do iter, err := and only set l.iter in the no error case, so we don't rely on newIter returning nil

[jbowens]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @aadityasondhi and @RaduBerinde)

---

internal/keyspan/keyspanimpl/level_iter.go line 158 at r2 (raw file):

Previously, RaduBerinde wrote…

[nit] maybe do iter, err := and only set l.iter in the no error case, so we don't rely on newIter returning nil

Good call

[RaduBerinde]

iterator.go line 1743 at r1 (raw file):

		i.rangeKey.updated = i.rangeKey.hasRangeKey && !i.Valid() && i.opts.rangeKeys()
	}
	// NextPrefix from an exhausted position is undefined. We keep the exhausted

Iterator.NextPrefix says you can't call it in IterAtLimit state but doesn't say anything about an exhausted iterator. If it's exhausted in the backward direction, isn't NextPrefix valid?

[jbowens]

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions

---

iterator.go line 1743 at r1 (raw file):

Previously, RaduBerinde wrote…

Iterator.NextPrefix says you can't call it in IterAtLimit state but doesn't say anything about an exhausted iterator. If it's exhausted in the backward direction, isn't NextPrefix valid?

Hrm yeah it seems like we could allow it as an operation semantically equivalent to a plain Next, but I also don't think we require it. Maybe this implementation should literally call Iterator.Next if exhausted.