alamb commented on code in PR #7670:
URL: https://github.com/apache/arrow-rs/pull/7670#discussion_r2150560713
##########
parquet-variant/Cargo.toml:
##########
@@ -33,5 +33,7 @@ rust-version = { workspace = true }
[dependencies]
arrow-schema = "55.1.0"
chrono = { workspace = true }
+serde_json = "1.0"
Review Comment:
It would be nice if we could avoid adding these dependencies if possible as
serdejson brings non trivial code.
This probably looks like adding a optional feature flag (like serde-json) or
something
I think we can do it as a follow on PR though
##########
parquet-variant/src/variant.rs:
##########
@@ -337,9 +405,15 @@ impl<'m, 'v> VariantArray<'m, 'v> {
}
pub fn values(&self) -> Result<impl Iterator<Item = Variant<'m, 'v>>,
ArrowError> {
- todo!();
- #[allow(unreachable_code)] // Just to infer the return type
- Ok(vec![].into_iter())
+ let len = self.len();
Review Comment:
This appears to conflict somewhat with
https://github.com/apache/arrow-rs/pull/7666 from @scovich
##########
parquet-variant/src/encoder/variant_to_json.rs:
##########
@@ -0,0 +1,652 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Module for converting Variant data to JSON format
+
+use arrow_schema::ArrowError;
+use base64::{Engine as _, engine::general_purpose};
+use serde_json::Value;
+use std::io::Write;
+
+use crate::variant::{Variant, VariantArray, VariantObject};
+
+/// Converts a Variant to JSON and writes it to the provided buffer
+///
+/// # Arguments
+///
+/// * `json_buffer` - Writer to output JSON to
+/// * `variant` - The Variant value to convert
+///
+/// # Returns
+///
+/// * `Ok(())` if successful
+/// * `Err` with error details if conversion fails
+///
+/// # Example
+///
+/// ```rust
+/// use parquet_variant::{Variant, variant_to_json};
+/// use arrow_schema::ArrowError;
+///
+/// fn example() -> Result<(), ArrowError> {
+/// let variant = Variant::Int8(42);
+/// let mut buffer = Vec::new();
+/// variant_to_json(&mut buffer, &variant)?;
+/// assert_eq!(String::from_utf8(buffer).unwrap(), "42");
+/// Ok(())
+/// }
+/// example().unwrap();
+/// ```
+pub fn variant_to_json<W: Write>(
+ json_buffer: &mut W,
+ variant: &Variant,
+) -> Result<(), ArrowError> {
+ match variant {
+ Variant::Null => {
+ write!(json_buffer, "null")?;
+ }
+ Variant::BooleanTrue => {
+ write!(json_buffer, "true")?;
+ }
+ Variant::BooleanFalse => {
+ write!(json_buffer, "false")?;
+ }
+ Variant::Int8(i) => {
+ write!(json_buffer, "{}", i)?;
+ }
+ Variant::Int16(i) => {
+ write!(json_buffer, "{}", i)?;
+ }
+ Variant::Int32(i) => {
+ write!(json_buffer, "{}", i)?;
+ }
+ Variant::Int64(i) => {
+ write!(json_buffer, "{}", i)?;
+ }
+ Variant::Float(f) => {
+ write!(json_buffer, "{}", f)?;
+ }
+ Variant::Double(f) => {
+ write!(json_buffer, "{}", f)?;
+ }
+ Variant::Decimal4 { integer, scale } => {
+ // Convert decimal to string representation
+ let divisor = 10_i32.pow(*scale as u32);
+ let decimal_value = *integer as f64 / divisor as f64;
+ write!(json_buffer, "{}", decimal_value)?;
+ }
+ Variant::Decimal8 { integer, scale } => {
+ // Convert decimal to string representation
+ let divisor = 10_i64.pow(*scale as u32);
+ let decimal_value = *integer as f64 / divisor as f64;
+ write!(json_buffer, "{}", decimal_value)?;
+ }
+ Variant::Decimal16 { integer, scale } => {
+ // Convert decimal to string representation
+ let divisor = 10_i128.pow(*scale as u32);
+ let decimal_value = *integer as f64 / divisor as f64;
+ write!(json_buffer, "{}", decimal_value)?;
+ }
+ Variant::Date(date) => {
+ write!(json_buffer, "\"{}\"", date.format("%Y-%m-%d"))?;
+ }
+ Variant::TimestampMicros(ts) => {
+ write!(json_buffer, "\"{}\"", ts.to_rfc3339())?;
+ }
+ Variant::TimestampNtzMicros(ts) => {
+ write!(json_buffer, "\"{}\"", ts.format("%Y-%m-%dT%H:%M:%S%.6f"))?;
+ }
+ Variant::Binary(bytes) => {
+ // Encode binary as base64 string
+ let base64_str = general_purpose::STANDARD.encode(bytes);
+ let json_str = serde_json::to_string(&base64_str)
+ .map_err(|e| ArrowError::InvalidArgumentError(format!("JSON
encoding error: {}", e)))?;
+ write!(json_buffer, "{}", json_str)?;
+ }
+ Variant::String(s) | Variant::ShortString(s) => {
+ // Use serde_json to properly escape the string
+ let json_str = serde_json::to_string(s)
+ .map_err(|e| ArrowError::InvalidArgumentError(format!("JSON
encoding error: {}", e)))?;
+ write!(json_buffer, "{}", json_str)?;
+ }
+ Variant::Object(obj) => {
+ convert_object_to_json(json_buffer, obj)?;
+ }
+ Variant::Array(arr) => {
+ convert_array_to_json(json_buffer, arr)?;
+ }
+ }
+ Ok(())
+}
+
+/// Convert object fields to JSON
+fn convert_object_to_json<W: Write>(
Review Comment:
This is very cool though I don't see any test coverage for writing Objects
to json -- can you perhaps add one?
--
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]
