rich7420 commented on code in PR #1402:
URL: https://github.com/apache/mahout/pull/1402#discussion_r3412339699
##########
qdp/qdp-core/src/readers/parquet.rs:
##########
@@ -561,10 +600,25 @@ impl<T: FloatElem> StreamingDataReader<T> for
ParquetStreamingReader<T> {
continue;
}
- let current_sample_size =
list_array.value_length(0) as usize;
-
- // Validate all rows in this batch have a
consistent sample size.
- for i in 1..list_array.len() {
+ // Find sample_size from the first non-null row.
+ // Null outer rows return value_length 0 and must
be skipped.
+ let first_non_null =
+ (0..list_array.len()).find(|&i|
!list_array.is_null(i));
+ let current_sample_size = match first_non_null {
+ Some(i) => list_array.value_length(i) as usize,
+ None => match self.sample_size {
+ // All rows null but sample_size known
from an earlier batch.
+ Some(ss) => ss,
+ // All rows null and sample_size unknown:
skip batch.
+ None => continue,
Review Comment:
**nit:** In `Reject` mode, when the first batch is entirely null and
`sample_size` is still unknown, `None => continue` returns before
`null_handling` is consulted — so this batch is silently skipped instead of
rejected. The batch `ParquetReader` errors on the same input, so the two
readers diverge here. It's also the one `Reject` path without test coverage.
Suggest either erroring on `Reject` here too, or documenting the difference
explicitly.
--
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]
