Question: Why are there 3 types of OffsetSizeTraits?

[HaoYang670]

Which part is this question about
In our code, there are 3 different OffsetSizeTrait: OffsetSizeTrait, BinaryOffsetSizeTrait and StringOffsetSizeTrait. The only difference between them is that BinaryOffsetSizeTrait and StringOffsetSizeTrait have a const dataType.

My questions are:

1. Why do we need three of them? Do we have to declare dataType as a const?
2. Could we just use one OffsetSizeTrait and get the dataType like this:

pub struct GenericBinaryArray<OffsetSize: OffsetSizeTrait> {
    data: ArrayData,
    value_offsets: RawPtrBox<OffsetSize>,
    value_data: RawPtrBox<u8>,
}

impl<OffsetSize: OffsetSizeTrait> GenericBinaryArray<OffsetSize> {
    ...
    pub fn get_datatype() -> DataType {
        if OffsetSize::is_large() {
            DataType::LargeBinary
        } else {
            DataType::Binary
        }
    }
    ...
}

[tustvold]

I don't see an obvious reason why they need to be separate traits, especially given that get_datatype() could be a const fn...

Perhaps @alamb or @jorgecarleitao remember some context of why these are the way they are?

[alamb]

Perhaps @alamb or @jorgecarleitao remember some context of why these are the way they are?

They pre-date my time with the project. Maybe @nevi-me or @andygrove or @paddyhoran have some memories or context to share

[jorgecarleitao]

I introduced them. At the time BinaryArray, LargeBinaryArray, StringArray and LargeStringArray were 4 different structs. The OffsetSizes were introduced to simplify the code via generics. At the time I did not realize that it could be even simpler. I agree that it can be simplified (arrow2 has only one).

[HaoYang670]

So I guess we can simplify the code by just reserving one offsetTrait and adding function const fn get_datatype() for GenericBInaryArray and GenericStringArray to get the DataType of the array.

[HaoYang670]

Close this issue as #1645 has been merged! Thank you for your all opinions!