Support ListView in length kernel

[vegarsti]

Which issue does this PR close?

Closes #9343.

Are these changes tested?

Yes, added tests

[Jefffrey]

Nice and simple 👍

[codephage2020]

The PR is good as-is for the length kernel. However, consider also adding bit_length support in a follow-up or in this same PR:

DataType::ListView(_) => {
    let list = array.as_list_view::<i32>();
    let v: Vec<i32> = list.sizes().iter().map(|s| s.wrapping_mul(8)).collect();
    Ok(Arc::new(Int32Array::new(v.into(), list.nulls().cloned())))
}
DataType::LargeListView(_) => {
    let list = array.as_list_view::<i64>();
    let v: Vec<i64> = list.sizes().iter().map(|s| s.wrapping_mul(8)).collect();
    Ok(Arc::new(Int64Array::new(v.into(), list.nulls().cloned())))
}

[vegarsti]

Nice one @codephage2020! Can do that here. Thanks for providing the code!

[Jefffrey]

Actually, I don't think it makes sense to support list types in bit_length 🤔

I know we have existing support for regular List/LargeList, but this seems to be introduced in error by #4718 as far as I can tell. It doesn't make sense to compute bit length of lists based on the assumption that each element of the list = 1 byte.

[vegarsti]

Actually, I don't think it makes sense to support list types in bit_length 🤔

I know we have existing support for regular List/LargeList, but this seems to be introduced in error by #4718 as far as I can tell. It doesn't make sense to compute bit length of lists based on the assumption that each element of the list = 1 byte.

Oh I see. Thanks. I'll remove it then.

[vegarsti]

Done!

[codephage2020]

Actually, I don't think it makes sense to support list types in bit_length 🤔
I know we have existing support for regular List/LargeList, but this seems to be introduced in error by #4718 as far as I can tell. It doesn't make sense to compute bit length of lists based on the assumption that each element of the list = 1 byte.

@Jefffrey Exactly. After reviewing the code, bit_length for List/LargeList multiplies element count by 8, which is semantically wrong since elements can be arbitrary types.

Oh I see. Thanks. I'll remove it then.

@vegarsti Could you also remove the existing List/LargeList support for bit_length?

[Jefffrey]

I've raised a separate issue for removing the existing implementation:

• Remove support for List types in bit_length kernel #9349

I think it would be better to leave this PR as improving support for ListViews only, and have a separate PR for a potentially behavioural change

[vegarsti]

To clarify, you still approve this @Jefffrey? 🙏🏻

[Jefffrey]

Yes

[Jefffrey]

Thanks @vegarsti & @codephage2020