You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "viirya (via GitHub)" <gi...@apache.org> on 2023/02/08 19:47:27 UTC

[GitHub] [arrow-rs] viirya commented on a diff in pull request #3676: Add ArrayData::new_null and DataType::primitive_width

viirya commented on code in PR #3676:
URL: https://github.com/apache/arrow-rs/pull/3676#discussion_r1100603418


##########
arrow-data/src/data.rs:
##########
@@ -667,83 +589,121 @@ impl ArrayData {
         &values.1[self.offset..]
     }
 
-    /// Returns a new empty [ArrayData] valid for `data_type`.
-    pub fn new_empty(data_type: &DataType) -> Self {
-        let buffers = new_buffers(data_type, 0);
-        let [buffer1, buffer2] = buffers;
-        let buffers = into_buffers(data_type, buffer1, buffer2);
-
-        let child_data = match data_type {
-            DataType::Null
-            | DataType::Boolean
-            | DataType::UInt8
-            | DataType::UInt16
-            | DataType::UInt32
-            | DataType::UInt64
-            | DataType::Int8
-            | DataType::Int16
-            | DataType::Int32
-            | DataType::Int64
-            | DataType::Float16
-            | DataType::Float32
-            | DataType::Float64
-            | DataType::Date32
-            | DataType::Date64
-            | DataType::Time32(_)
-            | DataType::Time64(_)
-            | DataType::Duration(_)
-            | DataType::Timestamp(_, _)
-            | DataType::Utf8
-            | DataType::Binary
-            | DataType::LargeUtf8
-            | DataType::LargeBinary
-            | DataType::Interval(_)
-            | DataType::FixedSizeBinary(_)
-            | DataType::Decimal128(_, _)
-            | DataType::Decimal256(_, _) => vec![],
-            DataType::List(field) => {
-                vec![Self::new_empty(field.data_type())]
-            }
-            DataType::FixedSizeList(field, _) => {
-                vec![Self::new_empty(field.data_type())]
-            }
-            DataType::LargeList(field) => {
-                vec![Self::new_empty(field.data_type())]
-            }
-            DataType::Struct(fields) => fields
-                .iter()
-                .map(|field| Self::new_empty(field.data_type()))
-                .collect(),
-            DataType::Map(field, _) => {
-                vec![Self::new_empty(field.data_type())]
-            }
-            DataType::Union(fields, _, _) => fields
-                .iter()
-                .map(|field| Self::new_empty(field.data_type()))
-                .collect(),
-            DataType::Dictionary(_, data_type) => {
-                vec![Self::new_empty(data_type)]
-            }
-            DataType::RunEndEncoded(run_ends, values) => {
-                vec![
-                    Self::new_empty(run_ends.data_type()),
-                    Self::new_empty(values.data_type()),
-                ]
-            }
+    /// Returns a new [`ArrayData`] valid for `data_type` containing `len` null values
+    pub fn new_null(data_type: &DataType, len: usize) -> Self {
+        let bit_len = bit_util::ceil(len, 8);
+        let zeroed = |len: usize| Buffer::from(MutableBuffer::from_len_zeroed(len));
+
+        let (buffers, child_data, has_nulls) = match data_type.primitive_width() {
+            Some(width) => (vec![zeroed(width * len)], vec![], true),
+            None => match data_type {
+                DataType::Null => (vec![], vec![], false),
+                DataType::Boolean => (vec![zeroed(bit_len)], vec![], true),
+                DataType::Binary | DataType::Utf8 => {
+                    (vec![zeroed((len + 1) * 4), zeroed(0)], vec![], true)
+                }
+                DataType::LargeBinary | DataType::LargeUtf8 => {
+                    (vec![zeroed((len + 1) * 8), zeroed(0)], vec![], true)
+                }
+                DataType::FixedSizeBinary(i) => {
+                    (vec![zeroed(*i as usize * len)], vec![], true)
+                }
+                DataType::List(f) | DataType::Map(f, _) => (
+                    vec![zeroed((len + 1) * 4)],
+                    vec![ArrayData::new_empty(f.data_type())],
+                    true,
+                ),
+                DataType::LargeList(f) => (
+                    vec![zeroed((len + 1) * 8)],
+                    vec![ArrayData::new_empty(f.data_type())],
+                    true,
+                ),
+                DataType::FixedSizeList(f, list_len) => (
+                    vec![],
+                    vec![ArrayData::new_null(f.data_type(), *list_len as usize * len)],
+                    true,
+                ),
+                DataType::Struct(fields) => (
+                    vec![],
+                    fields
+                        .iter()
+                        .map(|f| Self::new_null(f.data_type(), len))
+                        .collect(),
+                    true,
+                ),
+                DataType::Dictionary(k, v) => (
+                    vec![zeroed(k.primitive_width().unwrap() * len)],
+                    vec![ArrayData::new_empty(v.as_ref())],
+                    true,
+                ),
+                DataType::Union(f, i, mode) => {
+                    let ids = Buffer::from_iter(std::iter::repeat(i[0]).take(len));

Review Comment:
   Is it correct? Why repeating same type id? I think this indicates all values in the UnionArray are from same child array (`id[0]`)? But looks like you still produce all children null 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