You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "adamreeve (via GitHub)" <gi...@apache.org> on 2026/08/25 04:36:53 UTC

[PR] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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

   # Which issue does this PR close?
   
   - Closes #10830.
   
   # Rationale for this change
   
   Allows reading and writing this data type with Parquet. This is particularly useful for working with Polars categorical types, which use a `Dictionary(UInt32, Utf8View)` representation.
   
   # What changes are included in this PR?
   
   * Add missing match arms to support downcasting these arrays to a binary array when writing Parquet.
   * Add support for `BinaryView` and `Utf8View` value types in `ByteArrayDictionaryReader` to allow reading this data back as the same type.
   
   # Are these changes tested?
   
   Yes, I've added a new unit test to cover this.
   
   # Are there any user-facing changes?
   
   Yes, this is new user-facing functionality.
   
   


-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on PR #10831:
URL: https://github.com/apache/arrow-rs/pull/10831#issuecomment-5419678145

   will take a second look tommorow 👍 


-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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


##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,47 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {

Review Comment:
   `DictionaryBuffer` only works with an `OffsetBuffer`, not a `ViewBuffer`, so this extra conversion step is required: https://github.com/apache/arrow-rs/blob/3577093f26591aee806a9133c891b6b8e5255477/parquet/src/arrow/buffer/dictionary_buffer.rs#L42
   
   Maybe `DictionaryBuffer` could be refactored to avoid this but that seems like it would be a much more complex 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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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


Re: [PR] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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


##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,50 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {
+    let ArrowType::Dictionary(_, to_value_type) = to_type else {
+        return Err(general_err!(
+            "Cannot convert {} dictionary values to non-dictionary type {}",
+            array.data_type(),
+            to_type
+        ));
+    };
+
+    let data = array.to_data();
+    let values = make_array(data.child_data()[0].clone());
+
+    let new_values: ArrayRef = match to_value_type.as_ref() {
+        ArrowType::Utf8View => Arc::new(StringViewArray::from(values.as_string::<i32>())),
+        ArrowType::BinaryView => Arc::new(BinaryViewArray::from(values.as_binary::<i32>())),
+        other => {
+            return Err(general_err!(
+                "Cannot convert dictionary values to {}",
+                other
+            ));
+        }
+    };
+
+    let new_data = unsafe {
+        // safety: The original ArrayData is valid, and we only replace the child dictionary
+        // values with a correctly typed array, so skip ArrayDataBuilder validation.
+        data.into_builder()
+            .data_type(to_type.clone())
+            .child_data(vec![new_values.to_data()])
+            .build_unchecked()
+    };
+
+    Ok(make_array(new_data))

Review Comment:
   :+1: thanks, that's much simpler!



-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10831:
URL: https://github.com/apache/arrow-rs/pull/10831#issuecomment-5433458691

   thanks @adamreeve & @Rich-T-kid 


-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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


##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,47 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {

Review Comment:
   `DictionaryBuffer` only works with an `OffsetBuffer`, not a `ViewBuffer`, so this extra conversion step is required: https://github.com/apache/arrow-rs/blob/3577093f26591aee806a9133c891b6b8e5255477/parquet/src/arrow/buffer/dictionary_buffer.rs#L42
   
   Maybe `DictionaryBuffer` could be refactored to avoid this but that seems like it would be a much more complex change, and this probably isn't too performance sensitive as it only needs to convert the dictionary values, not the indices.



-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

Posted by "adamreeve (via GitHub)" <gi...@apache.org>.
adamreeve commented on PR #10831:
URL: https://github.com/apache/arrow-rs/pull/10831#issuecomment-5417797057

   Thanks for taking a look @Rich-T-kid, I've pushed a change to remove the ArrayDataBuilder validation.


-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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


##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,47 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {

Review Comment:
   `DictionaryBuffer` only works with an `OffsetBuffer`, not a `ViewBuffer`, so this extra conversion step is required: https://github.com/apache/arrow-rs/blob/3577093f26591aee806a9133c891b6b8e5255477/parquet/src/arrow/buffer/dictionary_buffer.rs#L42
   
   Maybe `DictionaryBuffer` could be refactored to avoid this or a new flavour of `DictionaryBuffer` could be created that works with view types, but that seems like it would be a much more complex change. This probably isn't too performance sensitive as it only needs to convert the dictionary values.
   
   I'm happy to look into making this work without the extra copy if we think that's needed though.



-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on code in PR #10831:
URL: https://github.com/apache/arrow-rs/pull/10831#discussion_r3854110781


##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,47 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {
+    let ArrowType::Dictionary(_, to_value_type) = to_type else {
+        return Err(general_err!(
+            "Cannot convert {} dictionary values to non-dictionary type {}",
+            array.data_type(),
+            to_type
+        ));
+    };
+
+    let data = array.to_data();
+    let values = make_array(data.child_data()[0].clone());
+
+    let new_values: ArrayRef = match to_value_type.as_ref() {
+        ArrowType::Utf8View => Arc::new(StringViewArray::from(values.as_string::<i32>())),
+        ArrowType::BinaryView => Arc::new(BinaryViewArray::from(values.as_binary::<i32>())),
+        other => {
+            return Err(general_err!(
+                "Cannot convert dictionary values to {}",
+                other
+            ));
+        }
+    };
+
+    let new_data = data
+        .into_builder()
+        .data_type(to_type.clone())
+        .child_data(vec![new_values.to_data()])
+        .build()?;
+
+    Ok(make_array(new_data))

Review Comment:
   I think we can avoid the extra validation that `ArrayDataBuilder` performs here.



##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,47 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {

Review Comment:
   yea after this is shipped I think this is worth making a follow up issue for to track.



-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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


##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,50 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {
+    let ArrowType::Dictionary(_, to_value_type) = to_type else {
+        return Err(general_err!(
+            "Cannot convert {} dictionary values to non-dictionary type {}",
+            array.data_type(),
+            to_type
+        ));
+    };
+
+    let data = array.to_data();
+    let values = make_array(data.child_data()[0].clone());
+
+    let new_values: ArrayRef = match to_value_type.as_ref() {
+        ArrowType::Utf8View => Arc::new(StringViewArray::from(values.as_string::<i32>())),
+        ArrowType::BinaryView => Arc::new(BinaryViewArray::from(values.as_binary::<i32>())),
+        other => {
+            return Err(general_err!(
+                "Cannot convert dictionary values to {}",
+                other
+            ));
+        }
+    };
+
+    let new_data = unsafe {
+        // safety: The original ArrayData is valid, and we only replace the child dictionary
+        // values with a correctly typed array, so skip ArrayDataBuilder validation.
+        data.into_builder()
+            .data_type(to_type.clone())
+            .child_data(vec![new_values.to_data()])
+            .build_unchecked()
+    };
+
+    Ok(make_array(new_data))

Review Comment:
   ```suggestion
       Ok(array.with_values(new_values))
   ```



##########
parquet/src/arrow/array_reader/byte_array_dictionary.rs:
##########
@@ -119,11 +123,50 @@ pub fn make_byte_array_dictionary_reader(
     }
 }
 
+/// Convert a dictionary-typed array with string or binary typed values
+/// to one with string or binary view typed values.
+fn convert_values_to_view(array: ArrayRef, to_type: &ArrowType) -> Result<ArrayRef> {
+    let ArrowType::Dictionary(_, to_value_type) = to_type else {
+        return Err(general_err!(
+            "Cannot convert {} dictionary values to non-dictionary type {}",
+            array.data_type(),
+            to_type
+        ));
+    };
+
+    let data = array.to_data();
+    let values = make_array(data.child_data()[0].clone());

Review Comment:
   ```suggestion
       let array = array.as_any_dictionary();
       let values = array.values();
   ```



-- 
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] feat(parquet): support round-trip of Dictionary(_, Utf8View/BinaryView) columns [arrow-rs]

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


-- 
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