alamb commented on code in PR #7783:
URL: https://github.com/apache/arrow-rs/pull/7783#discussion_r2167899808


##########
parquet-variant/src/variant.rs:
##########
@@ -32,6 +32,8 @@ mod decimal;
 mod list;
 mod metadata;
 mod object;
+use rust_decimal::prelude::*;
+use serde_json::Number;

Review Comment:
   I think we should be eventually be planning to avoid all use of serde_json 
for parsing / serializing. 
   
   It is fine to use serde_json for the initial version / getting things 
functionally working, but it is terribly inefficient compared to more optimized 
implementations like the tape decoder
   
   Thus can we please try and keep anything serde_json specific out of Variant 
(like this From impl)?



##########
parquet-variant/src/variant.rs:
##########
@@ -1031,6 +1033,57 @@ impl From<VariantDecimal16> for Variant<'_, '_> {
     }
 }
 
+impl TryFrom<&Number> for Variant<'_, '_> {
+    type Error = ArrowError;
+
+    fn try_from(n: &Number) -> Result<Self, Self::Error> {

Review Comment:
    follow up sounds like a good plan to me



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to