scovich commented on code in PR #8825:
URL: https://github.com/apache/arrow-rs/pull/8825#discussion_r2523588384
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -678,23 +678,32 @@ impl VariantToBinaryVariantArrowRowBuilder {
}
}
-struct FakeNullBuilder(NullArray);
+pub(crate) struct VariantToNullArrowRowBuilder {
+ capacity: usize,
+}
-impl FakeNullBuilder {
+impl VariantToNullArrowRowBuilder {
fn new(capacity: usize) -> Self {
- Self(NullArray::new(capacity))
+ Self { capacity }
}
- fn append_value<T>(&mut self, _: T) {}
- fn append_null(&mut self) {}
- fn finish(self) -> NullArray {
- self.0
+ fn append_value(&mut self, value: &Variant<'_, '_>) -> Result<bool> {
+ if let Some(_) = value.as_null() {
+ Ok(true)
+ } else {
+ // Null type only accepts nulls
Review Comment:
The comment in the issue about counting appended values/nulls was kind of
unrelated: _technically_ there's no guarantee the number of appends matches the
initial capacity estimate, which in arrow API is only an estimate. So we should
ideally have both `append_null` and `append_value` increment some counter,
whose final value dictates the size of the final array we create.
##########
parquet-variant-compute/src/variant_to_arrow.rs:
##########
@@ -678,23 +678,32 @@ impl VariantToBinaryVariantArrowRowBuilder {
}
}
-struct FakeNullBuilder(NullArray);
+pub(crate) struct VariantToNullArrowRowBuilder {
+ capacity: usize,
+}
-impl FakeNullBuilder {
+impl VariantToNullArrowRowBuilder {
fn new(capacity: usize) -> Self {
- Self(NullArray::new(capacity))
+ Self { capacity }
}
- fn append_value<T>(&mut self, _: T) {}
- fn append_null(&mut self) {}
- fn finish(self) -> NullArray {
- self.0
+ fn append_value(&mut self, value: &Variant<'_, '_>) -> Result<bool> {
+ if let Some(_) = value.as_null() {
+ Ok(true)
+ } else {
+ // Null type only accepts nulls
Review Comment:
Agree we can't add a NULL mask to a NullArray. But I thought the idea was to
let the normal strict-mode checking either error out or call (our fake)
`append_null` that actually does the same thing as `append_value` (namely
nothing). No null masks in sight.
--
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]
