You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "sdf-jkl (via GitHub)" <gi...@apache.org> on 2026/04/09 16:27:37 UTC
[PR] [Variant] Add `variant_get` access as `Variant` [arrow-rs]
sdf-jkl opened a new pull request, #9681:
URL: https://github.com/apache/arrow-rs/pull/9681
# 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 #8154.
# Rationale for this change
Check issue
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
- Added support for `variant_get` access as `Variant`
- Added unit tests
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are these changes tested?
Yes, added unit tests
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
# Are there any user-facing changes?
Can now extract unshredded `VariantArray` with `variant_get`
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
If there are any breaking changes to public APIs, please call them out.
-->
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3382818159
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -250,6 +266,18 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
Review Comment:
Nit: Could save a line by
```rust
ArrayRef::from(VariantArray::from_parts(
...
))
```
... but I didn't think `VariantArray` implemented `Array` (it cannot safely do so, lacking a `DataType::Variant` to pair with), so how can it be converted to `ArrayRef` at all?
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -250,6 +266,18 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
+ // Propagating metadata is not necessary for an all-NULL array, but is cheaper than constructing
+ // a new empty metadata array. (n * 3 bytes vs Arc bump)
+ input.metadata_field().clone(),
+ None,
+ None,
+ all_nulls,
+ );
Review Comment:
```suggestion
// Propagating metadata is not necessary for an all-NULL array, but is cheaper than constructing
// a new empty metadata array. (n * 3 bytes vs Arc bump)
let metadata = input.metadata_field().clone();
Ok(ArrayRef::from(VariantArray::from_parts(metadata, None, None, all_nulls)))
```
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -250,6 +266,18 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
Review Comment:
Ah! We have an `impl From<VariantArray> for ArrayRef` that returns the inner `StructArray`.
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -204,7 +204,23 @@ fn shredded_get_path(
// Helper that shreds a VariantArray to a specific type.
let shred_basic_variant =
|target: VariantArray, path: VariantPath<'_>, as_field: Option<&Field>| {
Review Comment:
This helper just became a lot more complicated, could use some inline code comments explaining how this new `requested_variant` stuff impacts the control flow?
e.g. "if the caller requested variant, unshred to binary it before continuing; if they also provided a path to traverse, don't pass a type so that the requested field can be fetched directly."
But that opens a bunch of questions:
1. Why unshred the whole thing if the user provided a non-empty `path`? Wouldn't it be better to any shredding as deeply as possible first? Or did the caller of `shred_basic_variant` already do that?
2. This code seems to ignore any shredded variant schema the user might have provided. I get that we don't necessarily want to tackle variant-get with custom shredding in this PR, but we should at least block it with an error instead of silently returning something different than was requested?
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -293,30 +321,30 @@ fn shredded_get_path(
//
// For shredded/partially-shredded targets (`typed_value` present), recurse into each field
// separately to take advantage of deeper shredding in child fields.
- if let DataType::Struct(fields) = as_field.data_type() {
- if target.typed_value_field().is_none() {
- return shred_basic_variant(target, VariantPath::default(), Some(as_field));
- }
-
- let children = fields
- .iter()
- .map(|field| {
- shredded_get_path(
- &target,
- &[VariantPathElement::from(field.name().as_str())],
- Some(field),
- cast_options,
- )
- })
- .collect::<Result<Vec<_>>>()?;
-
- let struct_nulls = target.nulls().cloned();
+ if !as_field.has_valid_extension_type::<VariantType>() {
+ if let DataType::Struct(fields) = as_field.data_type() {
+ if target.typed_value_field().is_none() {
+ return shred_basic_variant(target, VariantPath::default(), Some(as_field));
+ }
- return Ok(Arc::new(StructArray::try_new(
- fields.clone(),
- children,
- struct_nulls,
- )?));
+ let children = fields
+ .iter()
+ .map(|field| {
+ shredded_get_path(
+ &target,
+ &[VariantPathElement::from(field.name().as_str())],
+ Some(field),
+ cast_options,
+ )
Review Comment:
nit:
```suggestion
let path = &[VariantPathElement::from(field.name().as_str())];
shredded_get_path(&target, path, Some(field), cast_options)
```
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3397351260
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -204,7 +204,23 @@ fn shredded_get_path(
// Helper that shreds a VariantArray to a specific type.
let shred_basic_variant =
|target: VariantArray, path: VariantPath<'_>, as_field: Option<&Field>| {
Review Comment:
Fair! So we should just check for and block `typed_value` in the variant schema, to avoid any confusion.
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3319336144
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -171,6 +187,16 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
+ input.metadata_field().clone(),
Review Comment:
https://github.com/apache/arrow-rs/pull/9681/commits/406d84cad64f2de165c75fea3d2e6b105cf14072
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3397745533
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -204,7 +204,23 @@ fn shredded_get_path(
// Helper that shreds a VariantArray to a specific type.
let shred_basic_variant =
|target: VariantArray, path: VariantPath<'_>, as_field: Option<&Field>| {
Review Comment:
Done here https://github.com/apache/arrow-rs/pull/9681/commits/cf7e545f852623a6818bc520cf272711741a93a6. Sorry, should've split the comments/change/nit into separate commits
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3307560648
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -171,6 +187,16 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
+ input.metadata_field().clone(),
Review Comment:
Valid, but `metadata` array has to be the same length as Value/TypedValue fields.
By reusing `EMPTY_VARIANT_METADATA_BYTES: &[u8] = &[1, 0, 0];` we'll allocate 3 * array_len bytes vs doing Arc bump
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4566438116
@scovich fixed
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3397740471
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -250,6 +266,15 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ // Propagating metadata is not necessary for an all-NULL array, but is cheaper than constructing
+ // a new empty metadata array. (n * 3 bytes vs Arc bump)
+ let metadata = input.metadata_field().clone();
+ return Ok(ArrayRef::from(VariantArray::from_parts(
+ metadata, None, None, all_nulls,
+ )));
Review Comment:
https://github.com/apache/arrow-rs/pull/9681/commits/cf7e545f852623a6818bc520cf272711741a93a6
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4526467336
@scovich Please take a look when available!
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3397271791
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -204,7 +204,23 @@ fn shredded_get_path(
// Helper that shreds a VariantArray to a specific type.
let shred_basic_variant =
|target: VariantArray, path: VariantPath<'_>, as_field: Option<&Field>| {
Review Comment:
1. It does already do that.
2. Hmm... It's #8153.
I could do 2 but I'm not sure about `variant_get` custom shredding.
I checked Spark/duckdb/dremio `variant_get` implementations and it seems like no one does output shredding as a part of `variant_get` which is basically:
```rust
variant_get(arr, path, Some(shredding_schema)) ≡ shred_variant(variant_get(arr, path), schema)
```
There's a https://github.com/datafusion-contrib/datafusion-variant `Variant` integration and it strives for parity with spark/databricks.
Custom shredding will, however, skip unshred-reshred round trip that'd come from doing:
```rust
shred_variant(variant_get(arr, path), schema)
```
That's really a #8153 discussion
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4719330375
:shipit:
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4772152162
@alamb PTAL :sunglasses:
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3318985906
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -171,6 +187,16 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
+ input.metadata_field().clone(),
Review Comment:
Fair! Worth a code comment tho?
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4661908833
@scovich could you take a look 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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4772265934
I'll merge it on @scovich 's approval :) Any chance you can resolve the merge 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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3397271791
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -204,7 +204,23 @@ fn shredded_get_path(
// Helper that shreds a VariantArray to a specific type.
let shred_basic_variant =
|target: VariantArray, path: VariantPath<'_>, as_field: Option<&Field>| {
Review Comment:
1. It does already do that.
2. Hmm... It's #8154.
I could do 2 but I'm not sure about `variant_get` custom shredding.
I checked Spark/duckdb/dremio `variant_get` implementations and it seems like no one does output shredding as a part of `variant_get` which is basically:
```rust
variant_get(arr, path, Some(shredding_schema)) ≡ shred_variant(variant_get(arr, path), schema)
```
There's a https://github.com/datafusion-contrib/datafusion-variant `Variant` integration and it strives for parity with spark/databricks.
Custom shredding will, however, skip unshred-reshred round trip that'd come from doing:
```rust
shred_variant(variant_get(arr, path), schema)
```
That's really a #8154 discussion
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4565798403
Looks like there's a merge conflict?
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3306930658
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -171,6 +187,16 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
+ input.metadata_field().clone(),
Review Comment:
If it's all-NULL, why do we need to propagate the `metadata` column? Couldn't it be all-NULL as well, because the physical metadata values don't matter if the whole struct is masked by NULL?
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -214,30 +240,32 @@ fn shredded_get_path(
//
// For shredded/partially-shredded targets (`typed_value` present), recurse into each field
// separately to take advantage of deeper shredding in child fields.
- if let DataType::Struct(fields) = as_field.data_type() {
- if target.typed_value_field().is_none() {
- return shred_basic_variant(target, VariantPath::default(), Some(as_field));
- }
-
- let children = fields
- .iter()
- .map(|field| {
- shredded_get_path(
- &target,
- &[VariantPathElement::from(field.name().as_str())],
- Some(field),
- cast_options,
- )
- })
- .collect::<Result<Vec<_>>>()?;
-
- let struct_nulls = target.nulls().cloned();
+ if !as_field.has_valid_extension_type::<VariantType>() {
+ if let DataType::Struct(fields) = as_field.data_type() {
+ if target.typed_value_field().is_none() {
+ return shred_basic_variant(target, VariantPath::default(), Some(as_field));
+ }
- return Ok(Arc::new(StructArray::try_new(
- fields.clone(),
- children,
- struct_nulls,
- )?));
+ let children = fields
+ .iter()
+ .map(|field| {
+ shredded_get_path(
+ &target,
+ &[VariantPathElement::from(field.name().as_str())],
+ Some(field),
+ cast_options,
+ )
+ })
+ .collect::<Result<Vec<_>>>()?;
+
+ let struct_nulls = target.nulls().cloned();
+
+ return Ok(Arc::new(StructArray::try_new(
+ fields.clone(),
+ children,
+ struct_nulls,
Review Comment:
Why not just
```suggestion
return Ok(Arc::new(StructArray::try_new(
fields.clone(),
children,
target.nulls().cloned(),
```
--
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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4703981533
@scovich PTAL 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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#issuecomment-4782943111
gogogo
thanks 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] Add `variant_get` access as `Variant` [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3397365667
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -250,6 +266,15 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls = Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ // Propagating metadata is not necessary for an all-NULL array, but is cheaper than constructing
+ // a new empty metadata array. (n * 3 bytes vs Arc bump)
+ let metadata = input.metadata_field().clone();
+ return Ok(ArrayRef::from(VariantArray::from_parts(
+ metadata, None, None, all_nulls,
+ )));
Review Comment:
tiny nit:
```suggestion
let arr = VariantArray::from_parts(metadata, None, None, all_nulls);
return Ok(ArrayRef::from(arr));
```
(saves a line)
--
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