Transaction::GetIterator does not honor ReadOptions

[graetzer]

The TransactionBaseImpl::GetIterator does not honor the prefix_same_as_start and iterate_upper_bound options for keys written in the current transaction.

The underlying WBWIIteratorImpl does not check these options. This means the iterator returns non-matching writes from the current transaction, which we did not expect to see based on the documentation.

This should either be documented or fixed, otherwise it leads to some frustrating bugs.

[adamretter]

@graetzer Interesting. Would you be willing to contribute unit tests for this?

[graetzer]

@adamretter something like

rocksdb::Transaction* trx = tdb->BeginTransaction(wo, opts);
trx->Put("aaaa", "");
trx->Put("bbb", "");
trx->Put("ccc", "");
trx->Put("ddd", "");

std::string upper = "c";
rocksdb::ReadOptions ro;
ro.iterate_upper_bound = &upper;
auto it = trx->GetIterator(ro);
for (it->Seek("a"); it->Valid(); it->Next()) {
    assert(memcmp(it->key().data(), upper.data(), min(it->key().size(), upper.size())) < 0);
}

[gfosco]

Closing this via automation due to lack of activity. If you'd like this to be re-opened, please just comment here and we'll open it back up.

[evanmcc]

I ran into this as well and produced this test script that shows the iterator_upper_bound issue:
https://gist.github.com/evanmcc/da9769290f32f0861f5f99b5caa9ede2

If this is valid and you can tell me where a corresponding unit test would go, I'd be happy to submit a PR to add (obviously failing) test.

[siying]

@miasantreble worked upper bound support in transactions this in our last TechDebt week. We hit some problems and we haven't yet had time to finish it.

[evanmcc]

excellent. we can work around this for now, but it'd be lovely to have it over the longer term.

[gaoxiangxyz]

I meet this issue too.

[JayKickliter]

I ran into this bug as well. @evanmcc's reproduction is valid.

[adamretter]

See #7214