Rethink Cursor API

[CoreyKaylor]

There are scenarios today where Current vs. GetCurrent() can return different results. Re-evaluate the internals to possibly remove current key and value copy.

We should change methods to return MDBValue where appropriate, and still allow for IEnumerable / Current through the call to GetCurrent(). This avoids holding on to MDBValue copies internally to the cursor and avoids these oddities that make the internals become conditional when they don't need to be.

Proposed example:

private unsafe (bool found, MDBValue key, MDBValue value) Get(CursorOperation operation, ReadOnlySpan<byte> key)
{
       fixed (byte* keyPtr = key)
       {
           var mdbKey = new MDBValue(key.Length, keyPtr);
           var mdbValue = new MDBValue();
           return (mdb_cursor_get(_handle, ref mdbKey, ref mdbValue, operation) == 0, mdbKey, mdbValue);
       }
}

Or another alternative

private unsafe (int resultCode, MDBValue key, MDBValue value) Get(CursorOperation operation, ReadOnlySpan<byte> key)
{
       fixed (byte* keyPtr = key)
       {
           var mdbKey = new MDBValue(key.Length, keyPtr);
           var mdbValue = new MDBValue();
           return (mdb_cursor_get(_handle, ref mdbKey, ref mdbValue, operation), mdbKey, mdbValue);
       }
}

[CoreyKaylor]

@AlgorithmsAreCool curious on your thoughts here. I'll push up a PR and let you take a look, but I'm also inclined to re-evaluate return codes as well. Maybe provide an extension method for something like .Get(...).ThrowOnErrorCode().

I think it further simplifies the internal api going this route, and puts the choice on the end user for which route they prefer.

[AlgorithmsAreCool]

In short, It looks good. I'm also a big fan of providing return codes if they have useful info in them. I also like the idea of using extension methods against the tuple, it seems clean and inline friendly.

The rest of the Cursor API, I need to re-read. I recall finding some tension with the way we retained a copy of key/val and how the MDB APIworked, but i don't think i made notes at the time.

[CoreyKaylor]

The internals I'm already looking at and going this route would eliminate the wonkiness. Basically, it was capturing key/value internally so you could grab .Current / Iterate over the items. That said, this would still allow for existing external api usage with iteration / enumerable, but also gives you the key/value directly on the operations that it was capturing state for. Obviously a breaking change, but one that's pretty easily adapted.