You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tustvold (via GitHub)" <gi...@apache.org> on 2023/03/30 10:05:44 UTC

[GitHub] [arrow-rs] tustvold opened a new pull request, #3979: Add serde support to RawDecoder (#3949)

tustvold opened a new pull request, #3979:
URL: https://github.com/apache/arrow-rs/pull/3979

   _Draft as needs a load more tests_
   
   # Which issue does this PR close?
   
   <!--
   We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
   -->
   
   Closes #3949
   
   # Rationale for this change
    
   <!--
   Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
   Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
   -->
   
   Whilst this is necessarily inefficient, the UX of being able to take an arbitrary serde-compatible data structure and convert it to arrow is pretty compelling.
   
   # What changes are included in this PR?
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   # Are there any user-facing changes?
   
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!---
   If there are any breaking changes to public APIs, please add the `breaking change` label.
   -->
   


-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3979: Add serde support to RawDecoder (#3949)

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3979:
URL: https://github.com/apache/arrow-rs/pull/3979#discussion_r1154471050


##########
arrow-json/src/raw/mod.rs:
##########
@@ -233,6 +235,38 @@ impl RawDecoder {
         self.tape_decoder.decode(buf)
     }
 
+    /// Serialize `rows` to this [`RawDecoder`]

Review Comment:
   Added a fairly comprehensive example



-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-rs] tustvold merged pull request #3979: Support Rust structures --> `RecordBatch` by adding `Serde` support to `RawDecoder` (#3949)

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #3979:
URL: https://github.com/apache/arrow-rs/pull/3979


-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-rs] alamb commented on a diff in pull request #3979: Support Rust structures --> `RecordBatch` by adding `Serde` support to `RawDecoder` (#3949)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #3979:
URL: https://github.com/apache/arrow-rs/pull/3979#discussion_r1159963871


##########
arrow/src/lib.rs:
##########
@@ -271,6 +271,52 @@
 //!
 //! Parquet is published as a [separate crate](https://crates.io/crates/parquet)
 //!
+//! # Serde Compatibility

Review Comment:
   ❤️ 



-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-rs] alamb commented on a diff in pull request #3979: Add serde support to RawDecoder (#3949)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #3979:
URL: https://github.com/apache/arrow-rs/pull/3979#discussion_r1153235995


##########
arrow-json/src/raw/mod.rs:
##########
@@ -233,6 +235,38 @@ impl RawDecoder {
         self.tape_decoder.decode(buf)
     }
 
+    /// Serialize `rows` to this [`RawDecoder`]

Review Comment:
   I think a more compelling example would be to implement some struct 
   ```
   struct MyRow  {
     field: i32,
     name:  String
   }
   ```
   And demonstrate how to turn that into a `RecordBatch`



-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3979: Add serde support to RawDecoder (#3949)

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3979:
URL: https://github.com/apache/arrow-rs/pull/3979#discussion_r1153035655


##########
arrow-json/src/raw/serializer.rs:
##########
@@ -0,0 +1,395 @@
+use crate::raw::tape::TapeElement;
+use lexical_core::FormattedSize;
+use serde::ser::{
+    Impossible, SerializeMap, SerializeSeq, SerializeStruct, SerializeTuple,
+    SerializeTupleStruct,
+};
+use serde::{Serialize, Serializer};
+
+#[derive(Debug)]
+pub struct SerializerError(String);
+
+impl std::error::Error for SerializerError {}
+
+impl std::fmt::Display for SerializerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+impl serde::ser::Error for SerializerError {
+    fn custom<T>(msg: T) -> Self
+    where
+        T: std::fmt::Display,
+    {
+        Self(msg.to_string())
+    }
+}
+
+/// [`Serializer`] for [`TapeElement`]
+///
+/// Heavily based on <https://serde.rs/impl-serializer.html>
+pub struct TapeSerializer<'a> {
+    elements: &'a mut Vec<TapeElement>,
+
+    /// A buffer of parsed string data
+    bytes: &'a mut Vec<u8>,
+
+    /// Offsets into `data`
+    offsets: &'a mut Vec<usize>,
+}
+
+impl<'a> TapeSerializer<'a> {
+    pub fn new(
+        elements: &'a mut Vec<TapeElement>,
+        bytes: &'a mut Vec<u8>,
+        offsets: &'a mut Vec<usize>,
+    ) -> Self {
+        Self {
+            elements,
+            bytes,
+            offsets,
+        }
+    }
+}
+
+/// Need to use macro as const generic expressions are unstable
+/// <https://github.com/rust-lang/rust/issues/76560>
+macro_rules! serialize_lexical {
+    ($s:ident, $t:ty, $v:ident) => {{
+        let mut buffer = [0_u8; <$t>::FORMATTED_SIZE];
+        let s = lexical_core::write($v, &mut buffer);
+        $s.serialize_bytes(s)
+    }};
+}
+
+impl<'a, 'b> Serializer for &'a mut TapeSerializer<'b> {
+    type Ok = ();
+
+    type Error = SerializerError;
+
+    type SerializeSeq = ListSerializer<'a, 'b>;
+    type SerializeTuple = ListSerializer<'a, 'b>;
+    type SerializeTupleStruct = ListSerializer<'a, 'b>;
+    type SerializeTupleVariant = Impossible<(), SerializerError>;
+    type SerializeMap = ObjectSerializer<'a, 'b>;
+    type SerializeStruct = ObjectSerializer<'a, 'b>;
+    type SerializeStructVariant = Impossible<(), SerializerError>;
+
+    fn serialize_bool(self, v: bool) -> Result<(), SerializerError> {
+        self.elements.push(match v {
+            true => TapeElement::True,
+            false => TapeElement::False,
+        });
+        Ok(())
+    }
+
+    fn serialize_i8(self, v: i8) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i8, v)
+    }
+
+    fn serialize_i16(self, v: i16) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i16, v)
+    }
+
+    fn serialize_i32(self, v: i32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i32, v)
+    }
+
+    fn serialize_i64(self, v: i64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i64, v)
+    }
+
+    fn serialize_u8(self, v: u8) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u8, v)
+    }
+
+    fn serialize_u16(self, v: u16) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u16, v)
+    }
+
+    fn serialize_u32(self, v: u32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u32, v)
+    }
+
+    fn serialize_u64(self, v: u64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u64, v)
+    }
+
+    fn serialize_f32(self, v: f32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, f32, v)
+    }
+
+    fn serialize_f64(self, v: f64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, f64, v)

Review Comment:
   Formatting to a string only to parse it back again is rather wasteful, a future PR can likely tweak the tape representation to allow storing raw bytes



-- 
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-rs] alamb commented on a diff in pull request #3979: Support Rust structures --> `RecordBatch` by adding `Serde` support to `RawDecoder` (#3949)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #3979:
URL: https://github.com/apache/arrow-rs/pull/3979#discussion_r1156289716


##########
arrow-json/Cargo.toml:
##########
@@ -42,10 +42,12 @@ arrow-schema = { workspace = true  }
 half = { version = "2.1", default-features = false }
 indexmap = { version = "1.9", default-features = false, features = ["std"] }
 num = { version = "0.4", default-features = false, features = ["std"] }
+serde = { version = "1.0", default-features = false }

Review Comment:
   I assume this is not a net-new dependency as `serde_json` depends on `serde`



##########
arrow-json/src/raw/tape.rs:
##########
@@ -452,6 +454,26 @@ impl TapeDecoder {
         Ok(buf.len() - iter.len())
     }
 
+    pub fn serialize<S: Serialize>(&mut self, rows: &[S]) -> Result<(), ArrowError> {

Review Comment:
   I think we should add a doc string here explaining what this does. Something like
   
   ```suggestion
       /// Writes any struct that implements [`Serialize`] into the TapeDecoder
       /// so that it can be read as a RecordBatch
       pub fn serialize<S: Serialize>(&mut self, rows: &[S]) -> Result<(), ArrowError> {
   ```



##########
arrow-json/src/raw/serializer.rs:
##########
@@ -0,0 +1,417 @@
+// 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.
+
+use crate::raw::tape::TapeElement;
+use lexical_core::FormattedSize;
+use serde::ser::{
+    Impossible, SerializeMap, SerializeSeq, SerializeStruct, SerializeTuple,
+    SerializeTupleStruct,
+};
+use serde::{Serialize, Serializer};
+
+#[derive(Debug)]
+pub struct SerializerError(String);
+
+impl std::error::Error for SerializerError {}
+
+impl std::fmt::Display for SerializerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+impl serde::ser::Error for SerializerError {
+    fn custom<T>(msg: T) -> Self
+    where
+        T: std::fmt::Display,
+    {
+        Self(msg.to_string())
+    }
+}
+
+/// [`Serializer`] for [`TapeElement`]
+///
+/// Heavily based on <https://serde.rs/impl-serializer.html>
+pub struct TapeSerializer<'a> {
+    elements: &'a mut Vec<TapeElement>,
+
+    /// A buffer of parsed string data
+    bytes: &'a mut Vec<u8>,
+
+    /// Offsets into `data`
+    offsets: &'a mut Vec<usize>,
+}
+
+impl<'a> TapeSerializer<'a> {
+    pub fn new(
+        elements: &'a mut Vec<TapeElement>,
+        bytes: &'a mut Vec<u8>,
+        offsets: &'a mut Vec<usize>,
+    ) -> Self {
+        Self {
+            elements,
+            bytes,
+            offsets,
+        }
+    }
+}
+
+/// Need to use macro as const generic expressions are unstable
+/// <https://github.com/rust-lang/rust/issues/76560>
+macro_rules! serialize_lexical {
+    ($s:ident, $t:ty, $v:ident) => {{
+        let mut buffer = [0_u8; <$t>::FORMATTED_SIZE];
+        let s = lexical_core::write($v, &mut buffer);
+        $s.serialize_bytes(s)
+    }};
+}
+
+impl<'a, 'b> Serializer for &'a mut TapeSerializer<'b> {
+    type Ok = ();
+
+    type Error = SerializerError;
+
+    type SerializeSeq = ListSerializer<'a, 'b>;
+    type SerializeTuple = ListSerializer<'a, 'b>;
+    type SerializeTupleStruct = ListSerializer<'a, 'b>;
+    type SerializeTupleVariant = Impossible<(), SerializerError>;
+    type SerializeMap = ObjectSerializer<'a, 'b>;
+    type SerializeStruct = ObjectSerializer<'a, 'b>;
+    type SerializeStructVariant = Impossible<(), SerializerError>;
+
+    fn serialize_bool(self, v: bool) -> Result<(), SerializerError> {
+        self.elements.push(match v {
+            true => TapeElement::True,
+            false => TapeElement::False,
+        });
+        Ok(())
+    }
+
+    fn serialize_i8(self, v: i8) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i8, v)
+    }
+
+    fn serialize_i16(self, v: i16) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i16, v)
+    }
+
+    fn serialize_i32(self, v: i32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i32, v)
+    }
+
+    fn serialize_i64(self, v: i64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i64, v)
+    }
+
+    fn serialize_u8(self, v: u8) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u8, v)
+    }
+
+    fn serialize_u16(self, v: u16) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u16, v)
+    }
+
+    fn serialize_u32(self, v: u32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u32, v)
+    }
+
+    fn serialize_u64(self, v: u64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u64, v)
+    }
+
+    fn serialize_f32(self, v: f32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, f32, v)
+    }
+
+    fn serialize_f64(self, v: f64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, f64, v)
+    }
+
+    fn serialize_char(self, v: char) -> Result<(), SerializerError> {
+        self.serialize_str(&v.to_string())
+    }
+
+    fn serialize_str(self, v: &str) -> Result<(), SerializerError> {
+        self.serialize_bytes(v.as_bytes())
+    }
+
+    fn serialize_bytes(self, v: &[u8]) -> Result<(), SerializerError> {
+        self.bytes.extend_from_slice(v);
+        let idx = self.offsets.len() - 1;
+        self.elements.push(TapeElement::String(idx as _));
+        self.offsets.push(self.bytes.len());
+        Ok(())
+    }
+
+    fn serialize_none(self) -> Result<(), SerializerError> {
+        self.serialize_unit()
+    }
+
+    fn serialize_some<T>(self, value: &T) -> Result<(), SerializerError>
+    where
+        T: ?Sized + Serialize,
+    {
+        value.serialize(self)
+    }
+
+    fn serialize_unit(self) -> Result<(), SerializerError> {
+        self.elements.push(TapeElement::Null);
+        Ok(())
+    }
+
+    fn serialize_unit_struct(self, _name: &'static str) -> Result<(), SerializerError> {
+        self.serialize_unit()
+    }
+
+    fn serialize_unit_variant(
+        self,
+        _name: &'static str,
+        _variant_index: u32,
+        variant: &'static str,
+    ) -> Result<(), SerializerError> {
+        self.serialize_str(variant)
+    }
+
+    fn serialize_newtype_struct<T>(
+        self,
+        _name: &'static str,
+        value: &T,
+    ) -> Result<(), SerializerError>
+    where
+        T: ?Sized + Serialize,
+    {
+        value.serialize(self)
+    }
+
+    fn serialize_newtype_variant<T>(
+        self,
+        _name: &'static str,
+        _variant_index: u32,
+        variant: &'static str,
+        value: &T,
+    ) -> Result<(), SerializerError>
+    where
+        T: ?Sized + Serialize,
+    {
+        let mut serializer = self.serialize_map(Some(1))?;
+        serializer.serialize_key(variant)?;
+        serializer.serialize_value(value)?;
+        serializer.finish();
+        Ok(())
+    }
+
+    fn serialize_seq(
+        self,
+        _len: Option<usize>,
+    ) -> Result<Self::SerializeSeq, SerializerError> {
+        Ok(ListSerializer::new(self))
+    }
+
+    fn serialize_tuple(
+        self,
+        len: usize,
+    ) -> Result<Self::SerializeTuple, SerializerError> {
+        self.serialize_seq(Some(len))
+    }
+
+    fn serialize_tuple_struct(
+        self,
+        _name: &'static str,
+        len: usize,
+    ) -> Result<Self::SerializeTupleStruct, SerializerError> {
+        self.serialize_seq(Some(len))
+    }
+
+    fn serialize_tuple_variant(
+        self,
+        _name: &'static str,
+        _variant_index: u32,
+        _variant: &'static str,
+        _len: usize,
+    ) -> Result<Self::SerializeTupleVariant, SerializerError> {
+        Err(SerializerError(
+            "serializing tuple variants is not currently supported".to_string(),

Review Comment:
   It would be a better ux I think if the name and variant were in the message (same comment applies to `serialize_struct_variant`)
   
   ```suggestion
               format!("serializing tuple variants is not currently supported: {_name}::{_variant})")
   ```



##########
arrow-json/src/raw/mod.rs:
##########
@@ -233,6 +235,38 @@ impl RawDecoder {
         self.tape_decoder.decode(buf)
     }
 
+    /// Serialize `rows` to this [`RawDecoder`]

Review Comment:
   This example is very cool. However, I think it is very hard to find (given that it is on "RawDecoder" that is part of arrow_json). 
   
   I suggest we add a sentence / link to this example from the front page (and maybe even bring a simpler example to the front page):  https://docs.rs/arrow/latest/arrow/index.html#io



##########
arrow-json/src/raw/serializer.rs:
##########
@@ -0,0 +1,395 @@
+use crate::raw::tape::TapeElement;
+use lexical_core::FormattedSize;
+use serde::ser::{
+    Impossible, SerializeMap, SerializeSeq, SerializeStruct, SerializeTuple,
+    SerializeTupleStruct,
+};
+use serde::{Serialize, Serializer};
+
+#[derive(Debug)]
+pub struct SerializerError(String);
+
+impl std::error::Error for SerializerError {}
+
+impl std::fmt::Display for SerializerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+impl serde::ser::Error for SerializerError {
+    fn custom<T>(msg: T) -> Self
+    where
+        T: std::fmt::Display,
+    {
+        Self(msg.to_string())
+    }
+}
+
+/// [`Serializer`] for [`TapeElement`]
+///
+/// Heavily based on <https://serde.rs/impl-serializer.html>
+pub struct TapeSerializer<'a> {
+    elements: &'a mut Vec<TapeElement>,
+
+    /// A buffer of parsed string data
+    bytes: &'a mut Vec<u8>,
+
+    /// Offsets into `data`
+    offsets: &'a mut Vec<usize>,
+}
+
+impl<'a> TapeSerializer<'a> {
+    pub fn new(
+        elements: &'a mut Vec<TapeElement>,
+        bytes: &'a mut Vec<u8>,
+        offsets: &'a mut Vec<usize>,
+    ) -> Self {
+        Self {
+            elements,
+            bytes,
+            offsets,
+        }
+    }
+}
+
+/// Need to use macro as const generic expressions are unstable
+/// <https://github.com/rust-lang/rust/issues/76560>
+macro_rules! serialize_lexical {
+    ($s:ident, $t:ty, $v:ident) => {{
+        let mut buffer = [0_u8; <$t>::FORMATTED_SIZE];
+        let s = lexical_core::write($v, &mut buffer);
+        $s.serialize_bytes(s)
+    }};
+}
+
+impl<'a, 'b> Serializer for &'a mut TapeSerializer<'b> {
+    type Ok = ();
+
+    type Error = SerializerError;
+
+    type SerializeSeq = ListSerializer<'a, 'b>;
+    type SerializeTuple = ListSerializer<'a, 'b>;
+    type SerializeTupleStruct = ListSerializer<'a, 'b>;
+    type SerializeTupleVariant = Impossible<(), SerializerError>;
+    type SerializeMap = ObjectSerializer<'a, 'b>;
+    type SerializeStruct = ObjectSerializer<'a, 'b>;
+    type SerializeStructVariant = Impossible<(), SerializerError>;
+
+    fn serialize_bool(self, v: bool) -> Result<(), SerializerError> {
+        self.elements.push(match v {
+            true => TapeElement::True,
+            false => TapeElement::False,
+        });
+        Ok(())
+    }
+
+    fn serialize_i8(self, v: i8) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i8, v)
+    }
+
+    fn serialize_i16(self, v: i16) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i16, v)
+    }
+
+    fn serialize_i32(self, v: i32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i32, v)
+    }
+
+    fn serialize_i64(self, v: i64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, i64, v)
+    }
+
+    fn serialize_u8(self, v: u8) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u8, v)
+    }
+
+    fn serialize_u16(self, v: u16) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u16, v)
+    }
+
+    fn serialize_u32(self, v: u32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u32, v)
+    }
+
+    fn serialize_u64(self, v: u64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, u64, v)
+    }
+
+    fn serialize_f32(self, v: f32) -> Result<(), SerializerError> {
+        serialize_lexical!(self, f32, v)
+    }
+
+    fn serialize_f64(self, v: f64) -> Result<(), SerializerError> {
+        serialize_lexical!(self, f64, v)

Review Comment:
   ```suggestion
           // Formatting to a string only to parse it back again is rather wasteful, 
           // could likely tweak the tape representation to allow storing raw bytes
           serialize_lexical!(self, f64, v)
   ```



##########
arrow-json/src/raw/mod.rs:
##########
@@ -233,6 +235,143 @@ impl RawDecoder {
         self.tape_decoder.decode(buf)
     }
 
+    /// Serialize `rows` to this [`RawDecoder`]
+    ///
+    /// Whilst this provides a simple way to convert [`serde`]-compatible datastructures into arrow
+    /// [`RecordBatch`], performance-critical applications, especially where the schema is known at
+    /// compile-time, may benefit from instead implementing custom conversion logic as described
+    /// in [arrow_array::builder]
+    ///
+    /// This can be used with [`serde_json::Value`]
+    ///
+    /// ```
+    /// # use std::sync::Arc;
+    /// # use serde_json::{Value, json};
+    /// # use arrow_array::cast::AsArray;
+    /// # use arrow_array::types::Float32Type;
+    /// # use arrow_json::RawReaderBuilder;
+    /// # use arrow_schema::{DataType, Field, Schema};
+    /// let json = vec![json!({"float": 2.3}), json!({"float": 5.7})];
+    ///
+    /// let schema = Schema::new(vec![Field::new("float", DataType::Float32, true)]);
+    /// let mut decoder = RawReaderBuilder::new(Arc::new(schema)).build_decoder().unwrap();
+    ///
+    /// decoder.serialize(&json).unwrap();
+    /// let batch = decoder.flush().unwrap().unwrap();
+    /// assert_eq!(batch.num_rows(), 2);
+    /// assert_eq!(batch.num_columns(), 1);
+    /// let values = batch.column(0).as_primitive::<Float32Type>().values();
+    /// assert_eq!(values, &[2.3, 5.7])
+    /// ```
+    ///
+    /// It can also be used with arbitrary [`Serialize`] types

Review Comment:
   I recommend we also make a simpler example with no nested structures (maybe just an int32 and a string) in addition to the full featured example



-- 
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-unsubscribe@arrow.apache.org

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