[Discussion] Refactor the Decimals by using constant generic.

[HaoYang670]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
This is actually a question or a discussion.
Our current may to implement Decimal128 and Decimal256 is:

trait BasicDecimal: ... {
    const BIT_LENGTH: i32;
}

struct Decimal128 {}

struct Decimal256 {}

impl BasicDecimal for Decimal128 {}
impl BasicDecimal for Decimla256{}
impl Decimal128 {}

Actually I find an alternative way to implement this:

struct Decimal<const BYTE_LENGTH: usize> {
    precision: usize,
    scale: usize,
    value: [u8; BYTE_LENGTH],
}
impl<BYTE_LENGTH> Decimal<BYTE_LENGTH> {}

type Decimal128 = Decimal<16>;
type Decimal256 = Decimal<32>;

impl Decimal128 {}

I am not sure which one is better, but these are some pros and cons I thought:
pros: don't need the basic trait, reducing using macro when coding (better debugging)
cons: cannot restrict the bit_length to be 128 or 256 because const_generic_exprs is an unstable feature.

[HaoYang670]

cc @viirya. Looking forward to your opinions.

[HaoYang670]

related to #1999

[viirya]

Oh, I have considered generic. But as I cannot use 16, 32 as generic parameter, I don't go for this direction.

[HaoYang670]

But as I cannot use 16, 32 as generic parameter, I don't go for this direction.

Why can't 16, 32 be used as generic parameter? Rust has supported const generic

[viirya]

didn't you already say it is unstable feature?

[viirya]

Oh, that's const_generic_exprs. I don't know we can use const as generic parameter.

[HaoYang670]

didn't you already say it is unstable feature?
Oh, that's const_generic_exprs. I don't know we can use const as generic parameter.

Sorry for the confusion.
You are right. Theconst_generic is stable but the const_generic_exprs is ubstable, which means we can't add bound to the const generic so far:
(not tested)

#![feature(const_generic_exprs)]
impl<const BYTE_LENGTH> Decimal<{BYTE_LENGTH}> // const generic is stable
where {BYTE_LENGTH == 16 || BYTE_LENGTH == 32} // const generic expr is unstable
{...}

This is just an alternative way to implment Decimal, DecimalArray and DecimalArrayBuilder.

[HaoYang670]

@sunchao @alamb @tustvold Welcome to have a look and look forward to your opinions.

[tustvold]

Not fully thought through, but you could maybe do something like

struct Decimal128Type {}
struct Decimal256Type {}

trait DecimalType {
  ...
}

impl DecimalType for Decimal128Type {}
impl DecimalType for Decimal256Type {}

struct GenericDecimalArray<T: DecimalType> {}

type Decimal128Array = GenericDecimalArray<Decimal128Type>;
type Decimal256Array = GenericDecimalArray<Decimal256Type>;

[alamb]

I believe I remember that Decimal64 and Decimal32 were data types under discussion for addition to arrow, so making it easier to support additional lengths in the rust implementation would be great.

I looked at arrow2 for some inspiration but it appears to only support Decimal as a wrapper around i128

[HaoYang670]

Thank you for everyone's discussion! 😀
I will close this issue.
If anyone has some new ideas, we can reopen it.