You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "harshmotw-db (via GitHub)" <gi...@apache.org> on 2025/07/08 20:14:27 UTC
[PR] Harsh motwani data/parquet variant compute [arrow-rs]
harshmotw-db opened a new pull request, #7884:
URL: https://github.com/apache/arrow-rs/pull/7884
# 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.
- Closes #7883.
# Rationale for this change
Explained in the ticket.
**Note:** This PR will go through changes once [this PR](https://github.com/apache/arrow-rs/pull/7862) is merged.
# What changes are included in this PR?
This PR introduces two new functions `batch_json_string_to_variant` and `batch_variant_to_json_string` which can be used to transform batches of JSON strings to batches of Variant structs and vice versa. This PR attempts to implement `batch_variant_to_json_string` in a zero-copy way (@alamb see if you agree) since `variant_to_json` allows an input implementing a `Write` interface. `batch_json_string_to_variant` should also eventually be zero-copy once [this issue](https://github.com/apache/arrow-rs/issues/7805) is resolved.
# Are these changes tested?
Simple unit tests since the underlying functions have already been tested.
# Are there any user-facing changes?
Yes, it introduces the `batch_json_string_to_variant` and `batch_variant_to_json_string` APIs in a new crate.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193553633
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
+edition = { workspace = true }
+# needs a newer version than workspace due to
+# rror: `Option::<T>::unwrap` is not yet stable as a const fn
+rust-version = "1.83"
+
+
+[dependencies]
+arrow = { workspace = true }
+arrow-schema = { workspace = true }
+parquet-variant = { path = "../parquet-variant" }
Review Comment:
```
error: failed to parse manifest at `/Users/harsh.motwani/arrow-rs/parquet-variant-compute/Cargo.toml`
Caused by:
error inheriting `parquet-variant` from workspace root manifest's `workspace.dependencies.parquet-variant`
Caused by:
`dependency.parquet-variant` was not found in `workspace.dependencies`
```
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3062477505
Let's keep the train moving -- thanks again @harshmotw-db 🚀
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193379346
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
+edition = { workspace = true }
+# needs a newer version than workspace due to
+# rror: `Option::<T>::unwrap` is not yet stable as a const fn
+rust-version = "1.83"
Review Comment:
this rust-version difference might not be needed in this one
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
+edition = { workspace = true }
+# needs a newer version than workspace due to
+# rror: `Option::<T>::unwrap` is not yet stable as a const fn
+rust-version = "1.83"
+
+
+[dependencies]
+arrow = { workspace = true }
+arrow-schema = { workspace = true }
+parquet-variant = { path = "../parquet-variant" }
Review Comment:
Can you please make this "workspace=true" (otherwise cargo publish gets angsty)?
```suggestion
parquet-variant = { workspace = true }
```
##########
parquet-variant-compute/src/to_json.rs:
##########
@@ -0,0 +1,178 @@
+// 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 transforming a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY> into a batch of JSON strings.
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::DataType;
+use arrow_schema::ArrowError;
+use parquet_variant::{variant_to_json, Variant};
+
+pub fn batch_variant_to_json_string(input: &ArrayRef) -> Result<StringArray, ArrowError> {
+ let struct_array = input
+ .as_any()
+ .downcast_ref::<StructArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected StructArray as input".into()))?;
+
+ // Validate field types
+ let data_type = struct_array.data_type();
+ match data_type {
+ DataType::Struct(inner_fields) => {
+ if inner_fields.len() != 2
+ || inner_fields[0].data_type() != &DataType::Binary
+ || inner_fields[1].data_type() != &DataType::Binary
+ {
+ return Err(ArrowError::CastError(
+ "Expected struct with two binary fields".into(),
+ ));
+ }
+ }
+ _ => {
+ return Err(ArrowError::CastError(
+ "Expected StructArray with known fields".into(),
+ ))
+ }
+ }
+
+ let metadata_array = struct_array
+ .column(0)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'metadata'".into()))?;
+
+ let value_array = struct_array
+ .column(1)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'value'".into()))?;
+
+ // Zero-copy builder
+ // The size per JSON string is assumed to be 128 bytes. If this holds true, resizing could be
+ // minimized to improve performance.
+ let mut json_buffer: Vec<u8> = Vec::with_capacity(struct_array.len() * 128);
+ let mut offsets: Vec<i32> = Vec::with_capacity(struct_array.len() + 1);
+ let mut validity = BooleanBufferBuilder::new(struct_array.len());
+ let mut current_offset: i32 = 0;
+ offsets.push(current_offset);
+
+ for i in 0..struct_array.len() {
+ if struct_array.is_null(i) {
+ validity.append(false);
+ offsets.push(current_offset);
+ } else {
+ let metadata = metadata_array.value(i);
+ let value = value_array.value(i);
+ let variant = Variant::new(metadata, value);
+ let start_len = json_buffer.len();
+ variant_to_json(&mut json_buffer, &variant)?;
+ let written = (json_buffer.len() - start_len) as i32;
+ current_offset += written;
+ offsets.push(current_offset);
+ validity.append(true);
+ }
+ }
+
+ let offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(offsets));
+ let value_buffer = Buffer::from_vec(json_buffer);
+ let null_buffer = NullBuffer::new(validity.finish());
+
+ Ok(StringArray::new(
Review Comment:
I think if possible we should consider using StringViewArray here instead of StringArray
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
Review Comment:
We should definitely add some docs / example to this kernel
I also might suggest calling it `cast_to_variant` but that is more of a personal preference
##########
parquet-variant-compute/src/lib.rs:
##########
@@ -0,0 +1,27 @@
+// 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.
+
+mod from_json;
+mod to_json;
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
Review Comment:
Typically these comments would go on the function itself, not its `pub use`
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
Review Comment:
I think a placeholder that says "part of arrow-rs" and what it contains is probably enough
I would basically follow along the model of other crates in the repo
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
This will pattern will cause the variant values to be copied twice -- once into the builder's buffers and then once into the output binary builder, which is probably ok for the first version;
With some care I think we will be able to avoid copying the values, though it will take using the lower level APIs (and building offsets directly)
##########
parquet-variant-compute/src/to_json.rs:
##########
@@ -0,0 +1,178 @@
+// 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 transforming a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY> into a batch of JSON strings.
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::DataType;
+use arrow_schema::ArrowError;
+use parquet_variant::{variant_to_json, Variant};
+
+pub fn batch_variant_to_json_string(input: &ArrayRef) -> Result<StringArray, ArrowError> {
+ let struct_array = input
+ .as_any()
+ .downcast_ref::<StructArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected StructArray as input".into()))?;
+
+ // Validate field types
+ let data_type = struct_array.data_type();
+ match data_type {
+ DataType::Struct(inner_fields) => {
+ if inner_fields.len() != 2
+ || inner_fields[0].data_type() != &DataType::Binary
+ || inner_fields[1].data_type() != &DataType::Binary
+ {
+ return Err(ArrowError::CastError(
+ "Expected struct with two binary fields".into(),
+ ));
+ }
+ }
+ _ => {
+ return Err(ArrowError::CastError(
+ "Expected StructArray with known fields".into(),
+ ))
+ }
+ }
+
+ let metadata_array = struct_array
+ .column(0)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'metadata'".into()))?;
+
+ let value_array = struct_array
+ .column(1)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'value'".into()))?;
+
+ // Zero-copy builder
+ // The size per JSON string is assumed to be 128 bytes. If this holds true, resizing could be
+ // minimized to improve performance.
+ let mut json_buffer: Vec<u8> = Vec::with_capacity(struct_array.len() * 128);
+ let mut offsets: Vec<i32> = Vec::with_capacity(struct_array.len() + 1);
+ let mut validity = BooleanBufferBuilder::new(struct_array.len());
+ let mut current_offset: i32 = 0;
+ offsets.push(current_offset);
+
+ for i in 0..struct_array.len() {
+ if struct_array.is_null(i) {
+ validity.append(false);
+ offsets.push(current_offset);
+ } else {
+ let metadata = metadata_array.value(i);
+ let value = value_array.value(i);
+ let variant = Variant::new(metadata, value);
+ let start_len = json_buffer.len();
+ variant_to_json(&mut json_buffer, &variant)?;
+ let written = (json_buffer.len() - start_len) as i32;
+ current_offset += written;
+ offsets.push(current_offset);
Review Comment:
this looks good 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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "gotocoding-DB (via GitHub)" <gi...@apache.org>.
gotocoding-DB commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2194805401
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
+ let null_buffer = NullBuffer::new(validity.finish());
+ Ok(StructArray::new(
+ variant_fields,
+ struct_fields,
+ Some(null_buffer),
+ ))
+}
+
+#[cfg(test)]
+mod test {
+ use crate::batch_json_string_to_variant;
+ use arrow::array::{Array, ArrayRef, BinaryArray, StringArray};
+ use arrow_schema::ArrowError;
+ use parquet_variant::{Variant, VariantBuilder};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_batch_json_string_to_variant() -> Result<(), ArrowError> {
+ let input = StringArray::from(vec![
+ Some("1"),
+ None,
+ Some("{\"a\": 32}"),
+ Some("null"),
+ None,
+ ]);
Review Comment:
Can we please cover more cases here?
I mean all the types that JSON supports. I see you already added int, nulls, and simple dict here with string and int.
If it's test for happy-path can we please add:
* default values (e.g, `0` for `int` because some engines can represent NULL as a default value).
* booleans (both true/false)
* more nested json, not only 1-level nested json, e.g. `"{{{{true: false}, "-1": "+1"}, 0: 1}}"`
* some long strings to ensure we don't have any string size-based logic
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193605459
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
Well, would this resolve the issue you're talking about:
```
pub fn try_new(metadata: &'m [u8], value: &'v [u8]) -> Result<Self, ArrowError> {
// let metadata = VariantMetadata::try_new(metadata)?;
Self::try_new_with_metadata(VariantMetadata::try_new(metadata)?, value)
}
```
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195584723
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
Review Comment:
This was some debugging code I forgot to remove. Thanks for catching it.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195582311
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
+ let null_buffer = NullBuffer::new(validity.finish());
+ Ok(StructArray::new(
+ variant_fields,
+ struct_fields,
+ Some(null_buffer),
+ ))
+}
+
+#[cfg(test)]
+mod test {
+ use crate::batch_json_string_to_variant;
+ use arrow::array::{Array, ArrayRef, BinaryArray, StringArray};
+ use arrow_schema::ArrowError;
+ use parquet_variant::{Variant, VariantBuilder};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_batch_json_string_to_variant() -> Result<(), ArrowError> {
+ let input = StringArray::from(vec![
+ Some("1"),
+ None,
+ Some("{\"a\": 32}"),
+ Some("null"),
+ None,
+ ]);
Review Comment:
@gotocoding-DB The batch functions in this PR just run some underlying scalar functions on a whole batch of data. The underlying scalar functions have been validated on all sorts of inputs ([this PR](https://github.com/apache/arrow-rs/pull/7862)). I don't think the logical breadth of JSON test cases needs to be tested again.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193574002
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
Not sure I followed that one -- maybe it's a second issue?
I was just referring to all the `try_new_with_metadata` calls. Today `VariantMetadata` is `Copy`, so it's easy to forget that each such call is quietly copying an ~80 byte object. That cost could perhaps add up when iterating over hundreds/thousands of child objects? Or maybe the compiler is really good at making it ~free, since really only one meaningfully exists at a time?
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193555233
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
+edition = { workspace = true }
+# needs a newer version than workspace due to
+# rror: `Option::<T>::unwrap` is not yet stable as a const fn
+rust-version = "1.83"
+
+
+[dependencies]
+arrow = { workspace = true }
+arrow-schema = { workspace = true }
+parquet-variant = { path = "../parquet-variant" }
Review Comment:
Adding `parquet-variant = { path = "./parquet-variant" }` to the root Cargo.toml gets it working, so that's what I will do for now.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3063159737
> @alamb Sorry I was busy with something else yesterday. Thanks for merging it! What do you think about [this comment](https://github.com/apache/arrow-rs/pull/7884#issuecomment-3050486578) related to `BinaryViewArray` and `StringViewArray`?
I incorporated that suggestion it into this PR:
- https://github.com/apache/arrow-rs/pull/7905
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195594286
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
Review Comment:
Yeah, I agree with @scovich. As for `parse_json`, it makes sense to name it that way if it is a well defined SQL function. However, from a library point-of-view, I think `parse_json` is too vague.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193373037
##########
parquet-variant-compute/src/to_json.rs:
##########
@@ -0,0 +1,178 @@
+// 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 transforming a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY> into a batch of JSON strings.
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::DataType;
+use arrow_schema::ArrowError;
+use parquet_variant::{variant_to_json, Variant};
+
+pub fn batch_variant_to_json_string(input: &ArrayRef) -> Result<StringArray, ArrowError> {
+ let struct_array = input
+ .as_any()
+ .downcast_ref::<StructArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected StructArray as input".into()))?;
+
+ // Validate field types
+ let data_type = struct_array.data_type();
+ match data_type {
+ DataType::Struct(inner_fields) => {
+ if inner_fields.len() != 2
+ || inner_fields[0].data_type() != &DataType::Binary
+ || inner_fields[1].data_type() != &DataType::Binary
+ {
+ return Err(ArrowError::CastError(
+ "Expected struct with two binary fields".into(),
+ ));
+ }
+ }
+ _ => {
+ return Err(ArrowError::CastError(
+ "Expected StructArray with known fields".into(),
+ ))
+ }
+ }
+
+ let metadata_array = struct_array
+ .column(0)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'metadata'".into()))?;
+
+ let value_array = struct_array
+ .column(1)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'value'".into()))?;
+
+ // Zero-copy builder
+ // The size per JSON string is assumed to be 128 bytes. If this holds true, resizing could be
+ // minimized to improve performance.
Review Comment:
@alamb I've tried to minimize copying here. Took help from AI figuring this out. Please see if this looks good to you.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193634196
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
I have reduced copying in this function by manually constructing binary buffers.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193370393
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
Review Comment:
@alamb Let me know if I should create a readme.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193548615
##########
parquet-variant-compute/src/to_json.rs:
##########
@@ -0,0 +1,178 @@
+// 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 transforming a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY> into a batch of JSON strings.
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::DataType;
+use arrow_schema::ArrowError;
+use parquet_variant::{variant_to_json, Variant};
+
+pub fn batch_variant_to_json_string(input: &ArrayRef) -> Result<StringArray, ArrowError> {
+ let struct_array = input
+ .as_any()
+ .downcast_ref::<StructArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected StructArray as input".into()))?;
+
+ // Validate field types
+ let data_type = struct_array.data_type();
+ match data_type {
+ DataType::Struct(inner_fields) => {
+ if inner_fields.len() != 2
+ || inner_fields[0].data_type() != &DataType::Binary
+ || inner_fields[1].data_type() != &DataType::Binary
+ {
+ return Err(ArrowError::CastError(
+ "Expected struct with two binary fields".into(),
+ ));
+ }
+ }
+ _ => {
+ return Err(ArrowError::CastError(
+ "Expected StructArray with known fields".into(),
+ ))
+ }
+ }
+
+ let metadata_array = struct_array
+ .column(0)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'metadata'".into()))?;
+
+ let value_array = struct_array
+ .column(1)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .ok_or_else(|| ArrowError::CastError("Expected BinaryArray for 'value'".into()))?;
+
+ // Zero-copy builder
+ // The size per JSON string is assumed to be 128 bytes. If this holds true, resizing could be
+ // minimized to improve performance.
+ let mut json_buffer: Vec<u8> = Vec::with_capacity(struct_array.len() * 128);
+ let mut offsets: Vec<i32> = Vec::with_capacity(struct_array.len() + 1);
+ let mut validity = BooleanBufferBuilder::new(struct_array.len());
+ let mut current_offset: i32 = 0;
+ offsets.push(current_offset);
+
+ for i in 0..struct_array.len() {
+ if struct_array.is_null(i) {
+ validity.append(false);
+ offsets.push(current_offset);
+ } else {
+ let metadata = metadata_array.value(i);
+ let value = value_array.value(i);
+ let variant = Variant::new(metadata, value);
+ let start_len = json_buffer.len();
+ variant_to_json(&mut json_buffer, &variant)?;
+ let written = (json_buffer.len() - start_len) as i32;
+ current_offset += written;
+ offsets.push(current_offset);
+ validity.append(true);
+ }
+ }
+
+ let offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(offsets));
+ let value_buffer = Buffer::from_vec(json_buffer);
+ let null_buffer = NullBuffer::new(validity.finish());
+
+ Ok(StringArray::new(
Review Comment:
I don't know about that. According to [this documentation](https://github.com/apache/arrow-rs/blob/924687221f785a4b5ebb62fe2736ed47688900bf/arrow-array/src/array/byte_view_array.rs#L67-L79), the string must be stored in the offsets itself if it is short (<= 12 bytes). But how would we know that in advance? I think StringArray can more readily be implemented in a zero-copy fashion.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3055013636
I plan to give this another review tomorrow -- sorry I ran out of time today
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2198414052
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
> Oh, so just to be clear the two copies you are referring to are metadata_builder.append_value(&metadata); and metadata_builder.finish() right? If so, I'll take care of it in this PR itself.
What I was really imagining was updating VariantBuilder so it could take a pre-existing buffer (Vec) and append to it, rather than writing into a new buffer and then copying that into the output bytes.
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
+ let null_buffer = NullBuffer::new(validity.finish());
+ Ok(StructArray::new(
+ variant_fields,
+ struct_fields,
+ Some(null_buffer),
+ ))
+}
+
+#[cfg(test)]
+mod test {
+ use crate::batch_json_string_to_variant;
+ use arrow::array::{Array, ArrayRef, BinaryArray, StringArray};
+ use arrow_schema::ArrowError;
+ use parquet_variant::{Variant, VariantBuilder};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_batch_json_string_to_variant() -> Result<(), ArrowError> {
+ let input = StringArray::from(vec![
+ Some("1"),
+ None,
+ Some("{\"a\": 32}"),
+ Some("null"),
+ None,
+ ]);
Review Comment:
I agree full coverage here is redundant -- maybe we can add a comment that says "full json parsing coverage is handled by the tests for `json_to_variant`"
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
Review Comment:
Maybe we can call it `cast_json_to_variant` and `cast_variant_to_json` 🤔
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
Review Comment:
> But I guess we still have to push something to the offset arrays, to maintain proper positioning... so valid-but-empty is probably the best we can do?
This offset layout is required by the Arrow spec for StringArrays
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
Review Comment:
> Should variant_arrow_repr just return Fields instead of DataType? Then we don't have to unpack it.
Yes, please
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
Review Comment:
I agree we should allocate the normal vec and let it grow -- if we want to pre-allocate I think we should allow the user to pass in the allocation size as an argument
Maybe something like
```rust
let variant_array = JsonToVariant::new()
.with_capacity(...)
.parse(input_array)
```
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3050194224
@alamb @scovich @Samyak2
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193569175
##########
parquet-variant-compute/Cargo.toml:
##########
@@ -0,0 +1,46 @@
+# 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.
+
+[package]
+name = "parquet-variant-compute"
+# This package is still in development and thus the version does
+# not follow the versions of the rest of the crates in this repo.
+version = "0.1.0"
+license = { workspace = true }
+description = "Apache Parquet Variant Batch Processing"
+homepage = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+keywords = ["arrow", "parquet", "variant"]
+readme = "README.md"
Review Comment:
I just removed the readme line like some other crates in the repo.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "gotocoding-DB (via GitHub)" <gi...@apache.org>.
gotocoding-DB commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2194810331
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
Review Comment:
Can we please add not a happy-path test, for example, with `invalid json`, so this function should fail and return some well-known message to a user?
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195635045
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
Review Comment:
Well technically, it would be `parquet_variant_compute::parse_json`, which is a little clearer? Maybe we should add some nested module structure like arrow-compute does, so we get e.g. `parquet_variant_compute::variant::parse_json`. People would use that as `variant::parse_json` or `parse_json` if they prefer parsimony?
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "gotocoding-DB (via GitHub)" <gi...@apache.org>.
gotocoding-DB commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2194793047
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
Review Comment:
Don't we use `tracing` for logging in arrow?
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193437611
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
Oh, so just to be clear the two copies you are referring to are `metadata_builder.append_value(&metadata);` and `metadata_builder.finish()` right? If so, I'll take care of it in this PR itself.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195451441
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
Review Comment:
Where does the 128 scale factor come from? It seems a stretch to assume that a batch of `N` JSON strings will produce `128*N` bytes worth of unique field names? Especially as N grows and that scale factor becomes more and more dangerous?
If we really feel it's important to pre-allocate capacity, I'd recommend capping it at e.g. 1MB. But honestly, I'd just allocate a normal vec and let it grow normally, unless/until we have some proof that the guaranteed O(n) cost to append n bytes to a vec isn't good enough, and that pre-allocation actually helps in a wide variety of situations.
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
Review Comment:
We already know they're non-nullable... maybe the comment can explain that we're pushing valid-but-empty subfields, to maintain proper positioning?
Also: Could we create nullable sub-field arrays even tho the schema says they're non-nullable, and rely on nested null masks? Does that save space in case the variant column has a lot of null entries?
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
Review Comment:
As above... I'm not sure how helpful it really is to pre-allocate capacity based only on input length. Some variants will be very small, and an unbounded over-allocation could hog a lot of memory. Others will be much (much) larger than 128B each, and the vec will anyway end up making multiple capacity increases along the way.
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
Review Comment:
The shredding spec makes `value` optional, with the possibility -- but not the requirement -- to have a `typed_value` instead when `value` is missing. Should we recognize that situation and throw a suitable "shredded variants not supported" error, instead of blowing up with an obscure schema mismatch error?
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
Review Comment:
But I guess we still have to push _something_ to the offset arrays, to maintain proper positioning... so valid-but-empty is probably the best we can do?
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
Review Comment:
... but now that I think about it, that would be an issue when reading from parquet, which has nothing to do with this PR.
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
Review Comment:
Should `variant_arrow_repr` just return `Fields` instead of `DataType`? Then we don't have to unpack it.
It's also a 4-line method with a single call site (3 lines if we don't convert it to a `DataType`, so we might also consider removing the method entirely? Could also make the relationship with the `StructArray` more explicit by something like:
```suggestion
let metadata_field = Field::new("metadata", metadata_array.data_type(), false);
let value_field = Field::new("value", value_array.data_type(), false);
let variant_fields = vec![metadata_field, value_field].into();
```
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
+ let null_buffer = NullBuffer::new(validity.finish());
+ Ok(StructArray::new(
+ variant_fields,
+ struct_fields,
+ Some(null_buffer),
+ ))
+}
+
+#[cfg(test)]
+mod test {
+ use crate::batch_json_string_to_variant;
+ use arrow::array::{Array, ArrayRef, BinaryArray, StringArray};
+ use arrow_schema::ArrowError;
+ use parquet_variant::{Variant, VariantBuilder};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_batch_json_string_to_variant() -> Result<(), ArrowError> {
+ let input = StringArray::from(vec![
+ Some("1"),
+ None,
+ Some("{\"a\": 32}"),
+ Some("null"),
+ None,
+ ]);
+ let array_ref: ArrayRef = Arc::new(input);
+ let output = batch_json_string_to_variant(&array_ref).unwrap();
+
+ let struct_array = &output;
+ let metadata_array = struct_array
+ .column(0)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .unwrap();
+ let value_array = struct_array
+ .column(1)
+ .as_any()
+ .downcast_ref::<BinaryArray>()
+ .unwrap();
+
+ assert_eq!(struct_array.is_null(0), false);
+ assert_eq!(struct_array.is_null(1), true);
+ assert_eq!(struct_array.is_null(2), false);
+ assert_eq!(struct_array.is_null(3), false);
+ assert_eq!(struct_array.is_null(4), true);
+
+ assert_eq!(metadata_array.value(0), &[1, 0, 0]);
+ assert_eq!(value_array.value(0), &[12, 1]);
+
+ {
+ let mut vb = VariantBuilder::new();
+ let mut ob = vb.new_object();
+ ob.insert("a", Variant::Int8(32));
+ ob.finish()?;
+ let (object_metadata, object_value) = vb.finish();
+ assert_eq!(metadata_array.value(2), &object_metadata);
+ assert_eq!(value_array.value(2), &object_value);
+ }
+
+ assert_eq!(metadata_array.value(3), &[1, 0, 0]);
+ assert_eq!(value_array.value(3), &[0]);
+
+ // Ensure that the subfields are not actually nullable
Review Comment:
I'm not sure this is a useful thing to enforce? I'm pretty sure the arrow spec forbids to make any assumptions about the values of child arrays at positions an ancestor has marked invalid?
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
Review Comment:
Why not use `ok_or_else`?
```suggestion
let input_string_array = input
.as_any()
.downcast_ref::<StringArray>()
.ok_or_else(|| ArrowError::CastError("Expected a StringArray as input".into()))?;
```
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
+ let null_buffer = NullBuffer::new(validity.finish());
+ Ok(StructArray::new(
+ variant_fields,
+ struct_fields,
+ Some(null_buffer),
+ ))
+}
+
+#[cfg(test)]
+mod test {
+ use crate::batch_json_string_to_variant;
+ use arrow::array::{Array, ArrayRef, BinaryArray, StringArray};
+ use arrow_schema::ArrowError;
+ use parquet_variant::{Variant, VariantBuilder};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_batch_json_string_to_variant() -> Result<(), ArrowError> {
+ let input = StringArray::from(vec![
+ Some("1"),
+ None,
+ Some("{\"a\": 32}"),
+ Some("null"),
+ None,
+ ]);
Review Comment:
> default values
Not quite sure what you mean by "default values" or how an engine's NULL handling relates to string (json) -> variant parsing?
> `"{{{{true: false}, "-1": "+1"}, 0: 1}}"`
I'm pretty sure JSON objects requires string field names?
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
Review Comment:
I imagine we'll eventually want a top-level `cast_to_variant` that converts strong types to variant? And passing a string array to such a function should produce a variant column full of string (or short-string) variant values?
This method here is json-parsing strings and casting the result to variant, not casting strings directly to variant?
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3062230789
I merged this PR up to main and resolved some conflicts
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "gotocoding-DB (via GitHub)" <gi...@apache.org>.
gotocoding-DB commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2200112831
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,181 @@
+// 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 transforming a batch of JSON strings into a batch of Variants represented as
+//! STRUCT<metadata: BINARY, value: BINARY>
+
+use std::sync::Arc;
+
+use arrow::array::{Array, ArrayRef, BinaryArray, BooleanBufferBuilder, StringArray, StructArray};
+use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer};
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ let metadata_field = Field::new("metadata", DataType::Binary, false);
+ let value_field = Field::new("value", DataType::Binary, false);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+/// Parse a batch of JSON strings into a batch of Variants represented as
+/// STRUCT<metadata: BINARY, value: BINARY> where nulls are preserved. The JSON strings in the input
+/// must be valid.
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ // Zero-copy builders
+ let mut metadata_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut metadata_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut metadata_validity = BooleanBufferBuilder::new(input.len());
+ let mut metadata_current_offset: i32 = 0;
+ metadata_offsets.push(metadata_current_offset);
+
+ let mut value_buffer: Vec<u8> = Vec::with_capacity(input.len() * 128);
+ let mut value_offsets: Vec<i32> = Vec::with_capacity(input.len() + 1);
+ let mut value_validity = BooleanBufferBuilder::new(input.len());
+ let mut value_current_offset: i32 = 0;
+ value_offsets.push(value_current_offset);
+
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ // The subfields are expected to be non-nullable according to the parquet variant spec.
+ metadata_validity.append(true);
+ value_validity.append(true);
+ metadata_offsets.push(metadata_current_offset);
+ value_offsets.push(value_current_offset);
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
+ json_to_variant(input_string_array.value(i), &mut vb)?;
+ let (metadata, value) = vb.finish();
+ validity.append(true);
+
+ metadata_current_offset += metadata.len() as i32;
+ metadata_buffer.extend(metadata);
+ metadata_offsets.push(metadata_current_offset);
+ metadata_validity.append(true);
+
+ value_current_offset += value.len() as i32;
+ value_buffer.extend(value);
+ value_offsets.push(value_current_offset);
+ value_validity.append(true);
+ println!("{value_current_offset} {metadata_current_offset}");
+ }
+ }
+ let metadata_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(metadata_offsets));
+ let metadata_data_buffer = Buffer::from_vec(metadata_buffer);
+ let metadata_null_buffer = NullBuffer::new(metadata_validity.finish());
+
+ let value_offsets_buffer = OffsetBuffer::new(ScalarBuffer::from(value_offsets));
+ let value_data_buffer = Buffer::from_vec(value_buffer);
+ let value_null_buffer = NullBuffer::new(value_validity.finish());
+
+ let metadata_array = BinaryArray::new(
+ metadata_offsets_buffer,
+ metadata_data_buffer,
+ Some(metadata_null_buffer),
+ );
+ let value_array = BinaryArray::new(
+ value_offsets_buffer,
+ value_data_buffer,
+ Some(value_null_buffer),
+ );
+
+ let struct_fields: Vec<ArrayRef> = vec![Arc::new(metadata_array), Arc::new(value_array)];
+ let variant_fields = match variant_arrow_repr() {
+ DataType::Struct(fields) => fields,
+ _ => unreachable!("variant_arrow_repr is hard-coded and must match the expected schema"),
+ };
+ let null_buffer = NullBuffer::new(validity.finish());
+ Ok(StructArray::new(
+ variant_fields,
+ struct_fields,
+ Some(null_buffer),
+ ))
+}
+
+#[cfg(test)]
+mod test {
+ use crate::batch_json_string_to_variant;
+ use arrow::array::{Array, ArrayRef, BinaryArray, StringArray};
+ use arrow_schema::ArrowError;
+ use parquet_variant::{Variant, VariantBuilder};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_batch_json_string_to_variant() -> Result<(), ArrowError> {
+ let input = StringArray::from(vec![
+ Some("1"),
+ None,
+ Some("{\"a\": 32}"),
+ Some("null"),
+ None,
+ ]);
Review Comment:
@harshmotw-db You're right. If the internally used parsing function is tested, we don't need to duplicate the tests here. I'm personally using tests as another example of function usage (if it's not fully covered in function docs).
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3063060447
@alamb Sorry I was busy with something else yesterday. Thanks for merging it! What do you think about [this comment](https://github.com/apache/arrow-rs/pull/7884#issuecomment-3050486578) related to `BinaryViewArray` and `StringViewArray`?
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#issuecomment-3050486578
@alamb Wouldn't BinaryView have the same issues as the StringView issues I have highlighted [here](https://github.com/apache/arrow-rs/pull/7884#discussion_r2193548615)? I suppose implementing these `xxViewArrays` might be much more complicated to implement in a zero-copy manner. I'll leave the types as `Binary` and `String` for now unless I'm missing something.
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "harshmotw-db (via GitHub)" <gi...@apache.org>.
harshmotw-db commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2193605459
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
Well, would this resolve the issue you're talking about:
```
pub fn try_new(metadata: &'m [u8], value: &'v [u8]) -> Result<Self, ArrowError> {
// let metadata = VariantMetadata::try_new(metadata)?;
Self::try_new_with_metadata(VariantMetadata::try_new(metadata)?, value)
}
```
**Edit:** Oh no never mind. THere are more copies downstream. I suppose that is more of a library issue that can potentially be fixed separately
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
Well, would this resolve the issue you're talking about:
```
pub fn try_new(metadata: &'m [u8], value: &'v [u8]) -> Result<Self, ArrowError> {
// let metadata = VariantMetadata::try_new(metadata)?;
Self::try_new_with_metadata(VariantMetadata::try_new(metadata)?, value)
}
```
**Edit:** Oh no never mind. There are more copies downstream. I suppose that is more of a library issue that can potentially be fixed separately
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195423129
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
+ let input_string_array = match input.as_any().downcast_ref::<StringArray>() {
+ Some(string_array) => Ok(string_array),
+ None => Err(ArrowError::CastError(
+ "Expected reference to StringArray as input".into(),
+ )),
+ }?;
+
+ let mut metadata_builder = BinaryBuilder::new();
+ let mut value_builder = BinaryBuilder::new();
+ let mut validity = BooleanBufferBuilder::new(input.len());
+ for i in 0..input.len() {
+ if input.is_null(i) {
+ metadata_builder.append_null();
+ value_builder.append_null();
+ validity.append(false);
+ } else {
+ let mut vb = VariantBuilder::new();
Review Comment:
The above would definitely _not_ help, because each `VariantMetadata::try_new` would re-validate the same byte buffer!
--
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
Re: [PR] [Variant] Introduce parquet-variant-compute crate to transform batches of JSON strings to and from Variants [arrow-rs]
Posted by "Samyak2 (via GitHub)" <gi...@apache.org>.
Samyak2 commented on code in PR #7884:
URL: https://github.com/apache/arrow-rs/pull/7884#discussion_r2195441595
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -0,0 +1,136 @@
+// 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 std::sync::Arc;
+
+use arrow::array::{
+ Array, ArrayRef, BinaryBuilder, BooleanBufferBuilder, StringArray, StructArray,
+};
+use arrow::buffer::NullBuffer;
+use arrow::datatypes::{DataType, Field};
+use arrow_schema::ArrowError;
+use parquet_variant::{json_to_variant, VariantBuilder};
+
+fn variant_arrow_repr() -> DataType {
+ let metadata_field = Field::new("metadata", DataType::Binary, true);
+ let value_field = Field::new("value", DataType::Binary, true);
+ let fields = vec![metadata_field, value_field];
+ DataType::Struct(fields.into())
+}
+
+pub fn batch_json_string_to_variant(input: &ArrayRef) -> Result<StructArray, ArrowError> {
Review Comment:
`parse_json` is a name we could consider for this. It's the name of the function that does this (json string to variant) in spark/databricks.
--
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