Row changes #1787
Row changes #1787
Conversation
| /// Number of bytes required by the datum. | ||
| /// | ||
| /// This is used to optimistically pre-allocate buffers for packing rows. | ||
| pub fn datum_size(datum: &Datum) -> usize { |
There was a problem hiding this comment.
I'm not very happy with where this is at the moment. Because the row mod is private it can't be repr::row::datum_size(), and repr::datum_size() doesn't communicate what it is about. It could be a Row::datum_size() method?
There was a problem hiding this comment.
We could just make row public. As long as only the safe fns are public, should still be fine.
There was a problem hiding this comment.
Could also be Datum::packed_size
There was a problem hiding this comment.
Going to punt on this for the moment. @benesch indicates we might de-flatten repr at which point making it be a method in row would be great!
| pub struct Row { | ||
| data: Box<[u8]>, | ||
| } | ||
|
|
||
| /// These implementations order first by length, and then by slice contents. | ||
| /// This allows many comparisons to complete without dereferencing memory. |
There was a problem hiding this comment.
We should do it for Eq/PartialEq too.
There was a problem hiding this comment.
I hoped they were, but maybe they aren't. As in, I hoped Rust would be bright enough to check length equality first and then go for the contents. I'll look into it.
Minor changes to
Rowintended to uncontroversially improve things.