`StatisticsConverter::row_group_null_counts` incorrect for missing column about arrow-datafusion HOT 4 CLOSED

Should we also change the signature and return Result?

Yes I think so

Should we also change the signature and return Result?

I actually think using new_null_array is wrong here. It returns the wrong type -- for example if we are getting row counts for a Utf8 column, using new_null_arrray will return a StringArray rather than a UInt64Array.

I actually hit this issue when working on #10924

And I had to change it to

        let Some(parquet_index) = self.parquet_index else {
            let num_row_groups = metadatas.into_iter().count();
            return Ok(Arc::new(UInt64Array::from_iter(
                std::iter::repeat(None).take(num_row_groups),
            )));
        };

To make a test pass

from arrow-datafusion.

I actually think using new_null_array is wrong here. It returns the wrong type

I passed &DataType::UInt64 into new_null_array(...). However, your approach solves the issue with downcasting the result from new_null_array so it makes more sense here. Thank you.

@alamb just to confirm my understanding (regarding my second question) row_group_row_counts should also return a null_array when we reference a non-existing column; thus regardless if row group or data page both row_counts behave the same way when encountering a missing col?

from arrow-datafusion.

take

from arrow-datafusion.

@alamb
I have two questions here:

Should we also change the signature and return Result<Uint64Array>?
This however, would require to downcast_ref the result from new_null_array and clone the result.

Something like:

let Some(parquet_index) = self.parquet_index else {
            return Ok(self
                .make_null_array(&DataType::UInt64, metadatas)
                .as_any()
                .downcast_ref::<UInt64Array>()
                .expect("failed to downcast array")
                .clone());
        };

If we change the signature we also have to make some changes downstream, e.g. in row_groups.rs fn null_counts

Yet another question, what is the correct behaviour in data_page_row_counts if we cannot find a matching parquet_index, should we return a null_array (like we do already) or provide the row_group_row_count instead, since we cannot access the pages via parquet_index anyway? Is it even correct to not return a null-array in row_group_row_counts even if we can't match the column?

EDIT: I guess this answers the second question:

If there is not a column, the UInt64Array should be all nulls

from arrow-datafusion.