[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-05-06 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r421040067



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1174 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values
+/// are used to index into the `child_arrays`.
+///
+/// The `value_offsets` `Buffer` is only provided in the case of a dense 
union, sparse unions
+/// should use `None`.  If provided the `value_offsets` `Buffer` should 
contain `i32` values.
+/// These values should be greater than zero and must be less than the 
length of the overall
+/// array.
+///
+/// In both cases above we use signed integer types to maintain 
compatibility with other
+/// Arrow implementations.
+///
+/// In both of the 

[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-05-06 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r421038622



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1174 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values
+/// are used to index into the `child_arrays`.
+///
+/// The `value_offsets` `Buffer` is only provided in the case of a dense 
union, sparse unions
+/// should use `None`.  If provided the `value_offsets` `Buffer` should 
contain `i32` values.
+/// These values should be greater than zero and must be less than the 
length of the overall
+/// array.
+///
+/// In both cases above we use signed integer types to maintain 
compatibility with other
+/// Arrow implementations.
+///
+/// In both of the 

[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-05-06 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r421036704



##
File path: rust/arrow/src/buffer.rs
##
@@ -568,6 +568,25 @@ impl MutableBuffer {
 }
 }
 
+impl MutableBuffer {
+/// Writes a byte slice to the underlying buffer and updates the `len`, 
i.e. the
+/// number array elements in the builder.  Also, converts the `io::Result`
+/// required by the `Write` trait to the Arrow `Result` type.
+pub fn write_bytes( self, bytes: &[u8], len_added: usize) -> 
Result<()> {
+let write_result = self.write(bytes);
+// `io::Result` has many options one of which we use, so pattern 
matching is
+// overkill here
+if write_result.is_err() {
+Err(ArrowError::MemoryError(

Review comment:
   I'm fine with `IoError`, will change.





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.

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




[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-05-04 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r419658264



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1174 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values
+/// are used to index into the `child_arrays`.
+///
+/// The `value_offsets` `Buffer` is only provided in the case of a dense 
union, sparse unions
+/// should use `None`.  If provided the `value_offsets` `Buffer` should 
contain `i32` values.
+/// These values should be greater than zero and must be less than the 
length of the overall
+/// array.
+///
+/// In both cases above we use signed integer types to maintain 
compatibility with other
+/// Arrow implementations.
+///
+/// In both of the 

[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-05-04 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r419657778



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1174 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values
+/// are used to index into the `child_arrays`.
+///
+/// The `value_offsets` `Buffer` is only provided in the case of a dense 
union, sparse unions
+/// should use `None`.  If provided the `value_offsets` `Buffer` should 
contain `i32` values.
+/// These values should be greater than zero and must be less than the 
length of the overall
+/// array.
+///
+/// In both cases above we use signed integer types to maintain 
compatibility with other
+/// Arrow implementations.
+///
+/// In both of the 

[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-05-04 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r419653824



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1174 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values
+/// are used to index into the `child_arrays`.
+///
+/// The `value_offsets` `Buffer` is only provided in the case of a dense 
union, sparse unions
+/// should use `None`.  If provided the `value_offsets` `Buffer` should 
contain `i32` values.
+/// These values should be greater than zero and must be less than the 
length of the overall
+/// array.
+///
+/// In both cases above we use signed integer types to maintain 
compatibility with other
+/// Arrow implementations.
+///
+/// In both of the 

[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-04-23 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r414241189



##
File path: rust/arrow/src/array/mod.rs
##
@@ -85,6 +85,7 @@ mod array;
 mod builder;
 mod data;
 mod equal;
+mod union;

Review comment:
   Yea, even just to make the tests categorized better.  I'll open a JIRA 
once this is merged.





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.

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




[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-04-23 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r414240892



##
File path: rust/arrow/src/array/equal.rs
##
@@ -1046,6 +1062,30 @@ impl PartialEq for Value {
 }
 }
 
+impl JsonEqual for UnionArray {
+fn equals_json(, _json: &[]) -> bool {
+unimplemented!()
+}
+}
+
+impl PartialEq for UnionArray {
+fn eq(, json: ) -> bool {
+match json {
+Value::Array(json_array) => self.equals_json_values(_array),
+_ => false,
+}
+}
+}
+
+impl PartialEq for Value {
+fn eq(, arrow: ) -> bool {
+match self {

Review comment:
   Actually I was just following the pattern here, I thought that they must 
both be needed for IPC.  I removed them.  I'll add them back when I get to 
implementing IPC if needed.





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.

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




[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-04-22 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r412974468



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1172 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values
+/// are used to index into the `child_arrays`.
+///
+/// The `value_offsets` `Buffer` should contain `i32` values.  These 
values should be greater

Review comment:
   Yea, I only have a single constructor for both types that takes an 
`Option` for the `value_offsets`.  The comment should point this out, 
I'll update.





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 

[GitHub] [arrow] paddyhoran commented on a change in pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated

2020-04-22 Thread GitBox


paddyhoran commented on a change in pull request #7004:
URL: https://github.com/apache/arrow/pull/7004#discussion_r412972235



##
File path: rust/arrow/src/array/union.rs
##
@@ -0,0 +1,1172 @@
+// 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.
+
+//! Contains the `UnionArray` and `UnionBuilder` types.
+//!
+//! Each slot in a `UnionArray` can have a value chosen from a number of 
types.  Each of the
+//! possible types are named like the fields of a 
[`StructArray`](crate::array::StructArray).
+//! A `UnionArray` can have two possible memory layouts, "dense" or "sparse".  
For more information
+//! on please see the 
[specification](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
+//!
+//! Builders are provided for `UnionArray`'s involving primitive types.  
`UnionArray`'s of nested
+//! types are also supported but not via `UnionBuilder`, see the tests for 
examples.
+//!
+//! # Example: Dense Memory Layout
+//!
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_dense(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 0_i32);
+//! assert_eq!(union.value_offset(2), 1_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+//!
+//! # Example: Sparse Memory Layout
+//! ```
+//! use arrow::array::UnionBuilder;
+//! use arrow::datatypes::{Float64Type, Int32Type};
+//!
+//! # fn main() -> arrow::error::Result<()> {
+//! let mut builder = UnionBuilder::new_sparse(3);
+//! builder.append::("a", 1).unwrap();
+//! builder.append::("b", 3.0).unwrap();
+//! builder.append::("a", 4).unwrap();
+//! let union = builder.build().unwrap();
+//!
+//! assert_eq!(union.type_id(0), 0_i8);
+//! assert_eq!(union.type_id(1), 1_i8);
+//! assert_eq!(union.type_id(2), 0_i8);
+//!
+//! assert_eq!(union.value_offset(0), 0_i32);
+//! assert_eq!(union.value_offset(1), 1_i32);
+//! assert_eq!(union.value_offset(2), 2_i32);
+//!
+//! # Ok(())
+//! # }
+//! ```
+use crate::array::{
+builder::{builder_to_mutable_buffer, mutable_buffer_to_builder, 
BufferBuilderTrait},
+make_array, Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef,
+BooleanBufferBuilder, BufferBuilder, Int32BufferBuilder, Int8BufferBuilder,
+};
+use crate::buffer::{Buffer, MutableBuffer};
+use crate::datatypes::*;
+use crate::error::{ArrowError, Result};
+
+use crate::util::bit_util;
+use core::fmt;
+use std::any::Any;
+use std::collections::HashMap;
+use std::mem::size_of;
+
+/// An Array that can represent slots of varying types
+pub struct UnionArray {
+data: ArrayDataRef,
+boxed_fields: Vec,
+}
+
+impl UnionArray {
+/// Creates a new `UnionArray`.
+///
+/// Accepts type ids, child arrays and optionally offsets (for dense 
unions) to create
+/// a new `UnionArray`.  This method makes no attempt to validate the data 
provided by the
+/// caller and assumes that each of the components are correct and 
consistent with each other.
+/// See `try_new` for an alternative that validates the data provided.
+///
+/// # Data Consistency
+///
+/// The `type_ids` `Buffer` should contain `i8` values.  These values 
should be greater than
+/// zero and must be less than the number of children provided in 
`child_arrays`.  These values

Review comment:
   I was just going but 
[this](https://arrow.apache.org/docs/format/Columnar.html#union-layout).
   
   It's not immediately obvious why you would need that.  I'll have to study 
the C++ implementation.  I'll open a follow up PR.





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.

For queries about this service, please contact Infrastructure