scovich commented on code in PR #8392:
URL: https://github.com/apache/arrow-rs/pull/8392#discussion_r2364655465
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -23,15 +23,16 @@ use arrow::{
use arrow_schema::{ArrowError, DataType, FieldRef};
use parquet_variant::{VariantPath, VariantPathElement};
-use crate::variant_array::{ShreddedVariantFieldArray, ShreddingState};
+use crate::variant_array::ShreddingState;
use crate::variant_to_arrow::make_variant_to_arrow_row_builder;
use crate::VariantArray;
+use arrow::array::AsArray;
use std::sync::Arc;
-pub(crate) enum ShreddedPathStep<'a> {
+pub(crate) enum ShreddedPathStep {
/// Path step succeeded, return the new shredding state
- Success(&'a ShreddingState),
+ Success(ShreddingState),
Review Comment:
I mean, if somebody is concerned about cloning `ArrayRef` like it's free...
arrow-rs might be the wrong project for them to look very closely at?
Some potential ideas:
1. Have the caller of `variant_get` pass an owned `VariantArray`, and then
deconstruct it using some kind of `into_parts` method? Caller anyway had to
clone the various innards of the `&dyn Array` it would have started out with?
But that has the distinct disadvantage of producing a logically read-only
operation that consumes its input.
2. If we're ok with VariantArray being an exclusively local thing (i.e.
never returned from functions), we could make both it and `ShreddingState` take
references to their various arrays. That would produce `VariantArray<'a>`,
`ShreddingState<'a>`, etc.
3. Make `ShreddingState` track `Option<Cow<...>>` internally, which might
solve most of the ergonomics issues of 2/ without overly polluting the public
API. The only downside is a generic lifetime parameter: `VariantArray<'a>`,
`ShreddingState<'a>`, etc.
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
