Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions arrow-json/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ ryu = "1.0"
itoa = "1.0"

[dev-dependencies]
arrow-select = { workspace = true }
flate2 = { version = "1", default-features = false, features = ["rust_backend"] }
serde = { version = "1.0", default-features = false, features = ["derive"] }
futures = "0.3"
Expand Down
2 changes: 1 addition & 1 deletion arrow-json/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
pub mod reader;
pub mod writer;

pub use self::reader::{Reader, ReaderBuilder};
pub use self::reader::{ArrayDecoder, DecoderFactory, Reader, ReaderBuilder, Tape, TapeElement};
pub use self::writer::{
ArrayWriter, Encoder, EncoderFactory, EncoderOptions, LineDelimitedWriter, Writer,
WriterBuilder,
Expand Down
21 changes: 6 additions & 15 deletions arrow-json/src/reader/list_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
// specific language governing permissions and limitations
// under the License.

use crate::StructMode;
use crate::reader::tape::{Tape, TapeElement};
use crate::reader::{ArrayDecoder, make_decoder};
use crate::reader::{ArrayDecoder, DecoderContext};
use arrow_array::OffsetSizeTrait;
use arrow_array::builder::{BooleanBufferBuilder, BufferBuilder};
use arrow_buffer::buffer::NullBuffer;
Expand All @@ -34,27 +33,19 @@ pub struct ListArrayDecoder<O> {

impl<O: OffsetSizeTrait> ListArrayDecoder<O> {
pub fn new(
data_type: DataType,
coerce_primitive: bool,
strict_mode: bool,
ctx: &DecoderContext,
data_type: &DataType,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed all the decoder paths to pass borrowed refs because it was leading to a lot of clone calls otherwise.

Plus, one of the example decoders relies on the datatype instances to have stable memory locations during the recursive make_decoder call.

is_nullable: bool,
struct_mode: StructMode,
) -> Result<Self, ArrowError> {
let field = match &data_type {
let field = match data_type {
DataType::List(f) if !O::IS_LARGE => f,
DataType::LargeList(f) if O::IS_LARGE => f,
_ => unreachable!(),
};
let decoder = make_decoder(
field.data_type().clone(),
coerce_primitive,
strict_mode,
field.is_nullable(),
struct_mode,
)?;
let decoder = ctx.make_decoder(field.data_type(), field.is_nullable(), field.metadata())?;

Ok(Self {
data_type,
data_type: data_type.clone(),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leaf-level clone needed since we no longer pass owned DataType instances

decoder,
phantom: Default::default(),
is_nullable,
Expand Down
71 changes: 32 additions & 39 deletions arrow-json/src/reader/map_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
// specific language governing permissions and limitations
// under the License.

use crate::StructMode;
use crate::reader::tape::{Tape, TapeElement};
use crate::reader::{ArrayDecoder, make_decoder};
use crate::reader::{ArrayDecoder, DecoderContext};
use arrow_array::builder::{BooleanBufferBuilder, BufferBuilder};
use arrow_buffer::ArrowNativeType;
use arrow_buffer::buffer::NullBuffer;
Expand All @@ -33,46 +32,41 @@ pub struct MapArrayDecoder {

impl MapArrayDecoder {
pub fn new(
data_type: DataType,
coerce_primitive: bool,
strict_mode: bool,
ctx: &DecoderContext,
data_type: &DataType,
is_nullable: bool,
struct_mode: StructMode,
) -> Result<Self, ArrowError> {
let fields = match &data_type {
DataType::Map(_, true) => {
return Err(ArrowError::NotYetImplemented(
"Decoding MapArray with sorted fields".to_string(),
));
let DataType::Map(f, false) = data_type else {
return Err(ArrowError::NotYetImplemented(
"Decoding MapArray with sorted fields".to_string(),
));
};
Comment on lines -42 to +43
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opportunistic simplification


// TODO: Once MSRV bumps to 1.88+, use an if-let chain to directly unpack the slice
let fields = match f.data_type() {
DataType::Struct(fields) if fields.len() == 2 => fields,
d => {
return Err(ArrowError::InvalidArgumentError(format!(
"MapArray must contain struct with two fields, got {d}"
)));
}
DataType::Map(f, _) => match f.data_type() {
DataType::Struct(fields) if fields.len() == 2 => fields,
d => {
return Err(ArrowError::InvalidArgumentError(format!(
"MapArray must contain struct with two fields, got {d}"
)));
}
},
_ => unreachable!(),
};

let keys = make_decoder(
fields[0].data_type().clone(),
coerce_primitive,
strict_mode,
fields[0].is_nullable(),
struct_mode,
let (key_field, value_field) = (&fields[0], &fields[1]);

let keys = ctx.make_decoder(
key_field.data_type(),
key_field.is_nullable(),
key_field.metadata(),
)?;
let values = make_decoder(
fields[1].data_type().clone(),
coerce_primitive,
strict_mode,
fields[1].is_nullable(),
struct_mode,
let values = ctx.make_decoder(
value_field.data_type(),
value_field.is_nullable(),
value_field.metadata(),
)?;

Ok(Self {
data_type,
data_type: data_type.clone(),
keys,
values,
is_nullable,
Expand All @@ -82,12 +76,11 @@ impl MapArrayDecoder {

impl ArrayDecoder for MapArrayDecoder {
fn decode(&mut self, tape: &Tape<'_>, pos: &[u32]) -> Result<ArrayData, ArrowError> {
let s = match &self.data_type {
DataType::Map(f, _) => match f.data_type() {
s @ DataType::Struct(_) => s,
_ => unreachable!(),
},
_ => unreachable!(),
let DataType::Map(f, _) = &self.data_type else {
unreachable!()
};
let s @ DataType::Struct(_) = f.data_type() else {
unreachable!()
};

let mut offsets = BufferBuilder::<i32>::new(pos.len() + 1);
Expand Down
Loading
Loading