You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@arrow.apache.org by "L. C. Hsieh (Jira)" <ji...@apache.org> on 2022/05/31 17:16:00 UTC
[jira] [Created] (ARROW-16696) [C++]
L. C. Hsieh created ARROW-16696:
-----------------------------------
Summary: [C++]
Key: ARROW-16696
URL: https://issues.apache.org/jira/browse/ARROW-16696
Project: Apache Arrow
Issue Type: Improvement
Reporter: L. C. Hsieh
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 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.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)