You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Shekharrajak (via GitHub)" <gi...@apache.org> on 2026/05/12 05:00:01 UTC

[PR] feat: add VariantArrayBuilder::with_shredding [arrow-rs]

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

   
   
   # Rationale for this change
   
   Adds a builder method that configures VariantArrayBuilder to produce a shredded VariantArray at build() time.
   
   # What changes are included in this PR?
   Add shredding_schema: Option<DataType> field to VariantArrayBuilder
   Add with_shredding(as_type: DataType) -> Self — rows are still appended unshredded via append_variant; the shredding pass runs once at build() by delegating to shred_variant
   
   # Are these changes tested?
   
   unit tests 
   
   # Are there any user-facing changes?
   Yes, usage: 
   ```
   let arr = VariantArrayBuilder::new(3)
       .with_shredding(DataType::Int64)
       .tap(|b| { b.append_variant(Variant::Int64(42)); })
       .build(); // returns shredded VariantArray
   ```


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9963:
URL: https://github.com/apache/arrow-rs/pull/9963#issuecomment-5173856323

   Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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

   Hi @Shekharrajak, is this purely an ergonomic change? Would be cool to actually build a shredded array in one pass without building it first.
   


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9963: feat: add VariantArrayBuilder::with_shredding
URL: https://github.com/apache/arrow-rs/pull/9963


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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

   Updated as per review comments. 


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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


##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -659,4 +690,190 @@ mod test {
         assert_eq!(array.value(2), array2.value(2).get_list_element(0).unwrap());
         assert_eq!(array.value(2), array2.value(2).get_list_element(1).unwrap());
     }
+
+    #[test]
+    fn build_shredded_primitive_int64() {
+        let mut b = VariantArrayBuilder::new(3);
+        b.append_variant(Variant::Int64(42));
+        b.append_variant(Variant::Int64(100));
+        b.append_null();
+        let arr = b.build_shredded(&DataType::Int64).unwrap();
+        assert!(arr.typed_value_field().is_some());
+        assert_eq!(arr.len(), 3);
+        assert!(!arr.is_null(0));

Review Comment:
   Currently, we only assert whether the item is null, maybe we can assert the content of `arr` here.



##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -659,4 +690,190 @@ mod test {
         assert_eq!(array.value(2), array2.value(2).get_list_element(0).unwrap());
         assert_eq!(array.value(2), array2.value(2).get_list_element(1).unwrap());
     }
+
+    #[test]
+    fn build_shredded_primitive_int64() {
+        let mut b = VariantArrayBuilder::new(3);
+        b.append_variant(Variant::Int64(42));
+        b.append_variant(Variant::Int64(100));
+        b.append_null();
+        let arr = b.build_shredded(&DataType::Int64).unwrap();
+        assert!(arr.typed_value_field().is_some());
+        assert_eq!(arr.len(), 3);
+        assert!(!arr.is_null(0));

Review Comment:
   same as the tests below



-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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

   > Would be cool to actually build a shredded array in one pass without building it first
   
   Interesting! In _theory_ it should be possible with the shredded variant builders that `shred_variant` uses, but it would certainly take more than the current 10 LoC.


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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

   @alamb @scovich , please have a look. 


-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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


##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -145,7 +161,11 @@ impl VariantArrayBuilder {
         );
         // TODO add arrow extension type metadata
 
-        VariantArray::try_new(&inner).expect("valid VariantArray by construction")
+        let unshredded = VariantArray::try_new(&inner).expect("valid VariantArray by construction");
+        match shredding_schema {
+            Some(as_type) => shred_variant(&unshredded, &as_type).expect("shred_variant failed"),

Review Comment:
   Please no panics in prod code like this. Do we know what could go wrong? Does `build` need to return `Result` so we can surface the error properly?
   
   (I expect that the schema could be invalid, for example, which is technically a user error but an easy one to make if you don't fully understand the intricacies of variant shredding)



-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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


##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -659,4 +690,190 @@ mod test {
         assert_eq!(array.value(2), array2.value(2).get_list_element(0).unwrap());
         assert_eq!(array.value(2), array2.value(2).get_list_element(1).unwrap());
     }
+
+    #[test]
+    fn build_shredded_primitive_int64() {
+        let mut b = VariantArrayBuilder::new(3);
+        b.append_variant(Variant::Int64(42));
+        b.append_variant(Variant::Int64(100));
+        b.append_null();
+        let arr = b.build_shredded(&DataType::Int64).unwrap();
+        assert!(arr.typed_value_field().is_some());
+        assert_eq!(arr.len(), 3);
+        assert!(!arr.is_null(0));

Review Comment:
   Thanks for checking. Updated. 



-- 
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: add VariantArrayBuilder::with_shredding [arrow-rs]

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

   > if we call `with_shredding_schema` then return another array builder
   
   This looks better - updated commit : https://github.com/apache/arrow-rs/pull/9963/commits/b9bf55741bc5b333667c0f95d06fb6b52d8d6574 
   


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