klion26 commented on code in PR #10318:
URL: https://github.com/apache/arrow-rs/pull/10318#discussion_r3570133037
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -316,16 +325,27 @@ impl VariantArray {
})
}
+ /// Construct a perfectly shredded `VariantArray`: every value is in
+ /// `typed_value` and the required `value` column is all-null.
+ #[cfg(test)]
+ pub(crate) fn perfectly_shredded(
+ metadata: ArrayRef,
+ typed_value: ArrayRef,
+ nulls: Option<NullBuffer>,
+ ) -> Self {
+ let value = all_null_value_column(typed_value.len());
+ Self::from_parts(metadata, value, Some(typed_value), nulls)
+ }
+
pub(crate) fn from_parts(
metadata: ArrayRef,
- value: Option<ArrayRef>,
+ value: ArrayRef,
Review Comment:
What if we still keep the func signature, but `None` means perfect
shredding? so that the caller doesn't need to construct the all empty value
column
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -396,25 +416,22 @@ impl VariantArray {
/// Note: Does not do deep validation of the [`Variant`], so it is up to
the
/// caller to ensure that the metadata and value were constructed
correctly.
pub fn try_value(&self, index: usize) -> Result<Variant<'_, '_>> {
- match (self.typed_value_column(), self.value_column()) {
+ let value = self.value_column();
+ match self.typed_value_column() {
// Always prefer typed_value, if available
- (Some(typed_value), value) if typed_value.is_valid(index) => {
+ Some(typed_value) if typed_value.is_valid(index) => {
typed_value_to_variant(typed_value, value, index)
Review Comment:
Will moving the check in `typed_value_to_variant` here be better? so that we
don't need to pass the `value` column when calling`typed_value_to_variant`, the
fun name seems to be for `typed_value` but we still need to pass the `value` in
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -316,16 +325,27 @@ impl VariantArray {
})
}
+ /// Construct a perfectly shredded `VariantArray`: every value is in
+ /// `typed_value` and the required `value` column is all-null.
+ #[cfg(test)]
+ pub(crate) fn perfectly_shredded(
Review Comment:
Seems this is for better construct test cases, not sure if this is the right
place to put it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
