Do we need to public the MAX_DECIMAL_FOR_LARGER_PRECISION and MIN_DECIMAL_FOR_EACH_PRECISION

[liukun4515]

Which part is this question about

FYI #2320

For #2320, I will change the MIN and MAX array to bytes array. This will make a API break changes.

@viirya @alamb

Describe your question

Additional context

[liukun4515]

But in the preview release, we have exposed the
MAX_DECIMAL_FOR_EACH_PRECISION and MIN_DECIMAL_FOR_EACH_PRECISION

[alamb]

Hi @liukun4515

Are you suggesting that the constants such as MAX_DECIMAL_FOR_EACH_PRECISION etc should not be pub because you plan to change them in the future as part of #2320 ?

For anyone else who is interested, here is a link to the constants in question for your reference:
https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/datatypes/datatype.rs?L268-436&subtree=true

[liukun4515]

Hi @liukun4515

Are you suggesting that the constants such as MAX_DECIMAL_FOR_EACH_PRECISION etc should not be pub because you plan to change them in the future as part of #2320 ?

For anyone else who is interested, here is a link to the constants in question for your reference: https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/datatypes/datatype.rs?L268-436&subtree=true

Yes, we don't need to expose these two constant variables.
But I don't know why we expose the min and max for decimal128, maybe just missing them.

@alamb

[alamb]

it might be worth creating a PR with any proposed changes you have in mind so we can evaluate them.

Also, given we have been cranking out new major semver releases every 2 weeks I think it is ok if the API changes frequently in some of the areas that have more active churn and feature maturation like Decimal

[liukun4515]

it might be worth creating a PR with any proposed changes you have in mind so we can evaluate them.

Also, given we have been cranking out new major semver releases every 2 weeks I think it is ok if the API changes frequently in some of the areas that have more active churn and feature maturation like Decimal

👌

[viirya]

I think it is a minor change. Although these const values are exposed, I cannot think about it will be heavily depended. Sounds okay to hide them.

[tustvold]

I tried to make these private in #2711 (comment) but they are currently in use by DataFusion.

As more decimal logic gets moved to arrow-rs we may be able to remove these, but currently they are needed