Add read API without lifetimes

[cberner]

It would be nice to have a read transaction API that uses reference counting instead of lifetimes. This would allow callers to return the values/iterators retrieved from a table

[cberner]

@casey can you give the attached PR a try and let me know if that works for your use case?

[casey]

Dope, I'll give it a shot!

[casey]

I took a look at the PR, but it's big and it wasn't obvious to me how to use it.

What would the calls be to return an iterator from this function:

  pub(crate) fn get_home_inscriptions(&self) -> Result<Vec<InscriptionId>> {
    Ok(
      self
        .database
        .begin_read()?
        .open_table(HOME_INSCRIPTIONS)?
        .iter()?
        .rev()
        .flat_map(|result| result.map(|(_number, id)| InscriptionId::load(id.value())))
        .collect(),
    )
  }

[cberner]

Call range_arc(..) instead of iter() and you'll get an ArcRange instead of a Range. ArcRange has no lifetime and keeps its transaction alive, so you can return it. This test has a example of how to use it: https://github.com/cberner/redb/pull/732/files#diff-a475361d27182f73ec2c8436a78fe454a09b00ae07e6ea2186596598fa69b816R1263-R1268

[casey]

I finally got around to integrating this, and it works, with some caveats.

We read out of the database and then populate a template struct with the data that will be inserted into a HTML template.

This template object uses Display which, in combination with Axum, writes the contents to the request body.

We can get a query from the database as an impl Iterator, but then, since Display takes self by reference, we call iterate over the iterator without moving out of it.

The PR above gets around this by putting the iterator in an Mutex<Option<_>>, so in the template we can call self.inscriptions.lock().unwrap().take().unwrap(), which locks the mutex and removes the option from it. So overall this is really cool, but I don't think we'll be able to get much mileage out of it. If the iterator could implement a cheap .clone, then we could iterate over a clone.

[cberner]

Ah yes, I see. I'll take a look. I think it can impl Clone. Why don't you get much mileage from it as-is though? It looks like you're still able to avoid the collect()

[cberner]

Ok, give it a try now

[casey]

Just ran into an issue, in order for the iterator to be Clone, does the error type StorageError also need to be clone?

Got this error:

    Checking ord v0.15.0 (/Users/rodarmor/src/ord)
error: could not compile `ord` (lib) due to 2 previous errors
error[E0277]: the trait bound `StorageError: Clone` is not satisfied
    --> src/index.rs:1769:15
     |
1769 |   ) -> Result<impl Iterator<Item = InscriptionId> + Clone> {
     |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `StorageError`
     |
     = note: required for `std::result::Result<inscription_id::InscriptionId, StorageError>` to implement `Clone`
     = note: 1 redundant requirement hidden
     = note: required for `FlatMap<Rev<ArcRange<u32, (u128, u128, u32)>>, Result<..., ...>, ...>` to implement `Clone`
     = note: the full type name has been written to '/Users/rodarmor/src/ord/target/debug/deps/ord-a314c30b228b7ddc.long-type-10412504477855781909.txt'

[casey]

Here's the function:

  pub(crate) fn get_home_inscriptions(
    &self,
  ) -> Result<impl Iterator<Item = InscriptionId> + Clone> {
    Ok(
      self
        .database
        .begin_read()?
        .open_table(HOME_INSCRIPTIONS)?
        .range_arc::<u32>(..)?
        .rev()
        .flat_map(|result| result.map(|(_number, id)| InscriptionId::load(id.value()))),
    )
  }

[cberner]

Hmm, can you somehow call clone() before flat_map()? I don't think StorageError can be Clone. It contains an io::Error

[casey]

We could call clone before flat_map, but the issue is that we would ideally want to be able to call clone inside the template, so that it can clone the iterator and then iterate over eat, instead of doing the mutex/option/take stuff. Can ArcRange be clone? It seems like it should be possible, since it doesn't actually contain a storage error, but rustc is adamant that it doesn't work.

[cberner]

I refactored that PR to remove ArcRange and instead return Range<'static, K, V>. In 2.0 I'll probably be able to remove the lifetime from Range entirely. Ya, try this PR it adds Clone to Range: #745