0.10.0 Feedback

[casey]

Sorry about not looking at the tuple type stuff sooner! Just upgraded ord to 0.10.0 and wanted to give feedback on the upgrade.

• Ord indexing time went down by 10%/1.5 hours, almost certainly because it no longer zeroes pages. So nice improvement there.

• Not having to pass the max index size was actually a really nice improvement. We were previously exposing it as a flag, so not having to pass the max index size meant that we could remove that flag, a bunch of related argument parsing code, and plumbing it through to index creation. So now we don't have to do that, which let us delete a bunch of code and confusing command-line options.

• Functions like table.get and table.insert now no longer take a fixed type, so you can't do things like table.get(&key.into()), which is slightly annoying, but overall not a huge deal.

• the str alias is probably not super helpful. While adding & to key and value types, I was actually surprised I didn't have to do the same for str, so removing the str alias just for the sake of consistency is probably a good idea.

• Random thought: If RedbKey or RedbValue are implemented incorrectly, could this trigger UB in safe code? For example, if the types don't correspond correctly, or conversion to/from bytes is incorrect. If so, then they should be marked unsafe.

• Can get without a & be made to work? get and insert could take anything which implements Borrow, and T implements Borrow<T> (blanket impl in stdlib) so then you could do .get([0; 12]) and .get(&[0; 12]).

• I'm not entirely clear why some types now need & and some don't when used as keys and values. I think ideally, you wouldn't have to add the &, mostly because some types require it and some types don't. So understanding why[u8; 32] must be &[u8; 32] but [u8] can just be [u8] and not &[u8] is a bit of cognitive overhead.

[cberner]

No, worries!

Not having to pass the max index size was actually a really nice improvement.

Nice! Ya, I was very happy to have found a way to remove that

the str alias is probably not super helpful

Agreed. I have a TODO about removing those :)

Random thought: If RedbKey or RedbValue are implemented incorrectly, could this trigger UB in safe code?

Not that I can think of. Internally redb treats everything as opaque slices and calls into the compare method to order them. You certainly could get nonsensical results -- like inserting a value, reading back, and getting None -- but I don't think it would be UB

Ya, so the type stuff for get() and insert() is kinda annoying. I made this Playground to test out different argument types. I'm definitely open to changing it if you have an idea for making it better. I went with &Borrow because it seemed most similar to how std works.
I believe Borrow doesn't work for .get(&[0; 12]) because &[u8; 12] does not implement Borrow<[u8]> even though [u8; 12] does implement Borrow<[u8]>. Similarly I had trouble with Vec when I tried that solution. The following didn't work:

table = Table<&[u8], &[u8]>;
let x = vec![0u8];
table.get(&x);

[cberner]

#453 removes the [u8] and str alias types

[cberner]

I wrote up my plan for changing the argument types to make it consistent whether & is required or not in #452 . Lemme know if you think of a better option!