[ARROW-16696] [C++] Is Precision check necessary when appending values at decimal builder

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Won't Fix
• External issue URL: https://github.com/apache/arrow/issues/32040
• Assignee: Unassigned
• Reporter: L. C. Hsieh
• Created: 31/May/22 17:15
• Updated: 11/Jan/23 11:45
• Resolved: 03/Jun/22 16:15

Description

This is one issue found when I'm trying to enable generate_decimal128_case integration case for Rust at C++ repo: https://github.com/apache/arrow/pull/13219.

Rust validates precision of decimal in few places, e.g. DecimalBuilder's append_value. However, in decimal golden file 0.14.1_decimal.gold.json, there are some values failing this check. C++ doesn't perform similar check in appending value at decimal builder as I did a search (e.g. Decimal128Builder, DecimalFromString), and I can confirm that test case can be passed if I remove the check in DecimalBuilder.append_value.

Actually in C++, similar check is done when doing a full validating ArrayData: https://github.com/apache/arrow/blob/c715bebbd89089f385c9996560866da23ea1ddda/cpp/src/arrow/array/validate.cc#L672.

I'm wondering if we should follow C++ at Rust implementation, i.e. removing the check from appending value at decimal builder, and moving the check to the full validation of ArrayData?

I opened a Rust PR (https://github.com/apache/arrow-rs/pull/1767) for that.

Another opinion from the comments is, as we don't do full validation when finishing the decimal builder, it seems this check is necessary if it is the promise of the builder to always build a valid ArrayData. Then the check is needed and it seems C++ needs to add the check there.

Would like to hear some feedback here. Thanks.

Issue Links

• relates to: ARROW-13558 [C++] Validate decimal data in ValidateFull