alamb commented on code in PR #7809:
URL: https://github.com/apache/arrow-rs/pull/7809#discussion_r2173664270
##########
parquet-variant/src/variant.rs:
##########
@@ -629,6 +638,9 @@ impl<'m, 'v> Variant<'m, 'v> {
Variant::Int16(i) => Some(i.into()),
Variant::Int32(i) => Some(i.into()),
Variant::Int64(i) => Some(i),
+ Variant::Decimal4(d) if d.scale == 0 => Some(d.integer.into()),
+ Variant::Decimal8(d) if d.scale == 0 => Some(d.integer),
+ Variant::Decimal16(d) if d.scale == 0 => d.integer.try_into().ok(),
Review Comment:
One minor comment is that the `ok()` here discards an `ArowError` which has
an owned string -- so in othr words, this call is pretty expensive. It might be
worth a method that returns a `Option` rather than `Err` for testing conversion
The same general rule applies below
One potential solution woudl be to add methods like
```rust
struct VariantDecimal8 {
// returns Some if this variant can be represented as Decimal4, None
otherwise
fn as_decimal_4(&self) -> Option<VariantDecimal4> { ... }
--
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]
