alamb commented on issue #7700:
URL: https://github.com/apache/arrow-rs/issues/7700#issuecomment-2991217639
> The change looks great!
>
> > The string length check right before appending also seems unavoidable
because it's possible for users to construct `Variant::ShortString` directly,
without doing input validation.
>
> Good catch. That is indeed rather annoying. I guess we could potentially
mask off all but the lowest 6 bits of the string's length and write that. Then
the behavior is always well-defined?
I think this would be surprising (to silently truncate the values) and would
not recommend it
> Alternatively, we could make `Variant` be a struct whose only (private)
field is today's enum? That way, we could enforce always-valid `Variant` values
via `try_new`. But it would make variants harder to work with, because user
code could no longer match on the enum variants. Ugh.
Yeah I agree having `match` for Variant is really nice
Another idea would be to introduce a newtype similar to the `VariantObject`
struct that wraps a `&str` and ensures lengths are valid:
```rust
enum Variant {
...
// short string doesn't contain a `&str` but rather a `ShortString
ShortString(ShortString<'v>),
...
}
```
And then we would put validtaion on creating a ShortSstring
```rust
/// Wraps a string that is guaranteed to be a valid `Variant::ShortString`
struct ShortString<'a>(&'a str);
impl <'a> ShortString<'a> {
fn try_new(value: &str) -> Result<Self> {
// check value.len and return an error if too long
Self(value)
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
