pitrou commented on a change in pull request #11322:
URL: https://github.com/apache/arrow/pull/11322#discussion_r722307281
##########
File path: cpp/src/arrow/compute/kernels/scalar_cast_test.cc
##########
@@ -704,14 +706,15 @@ TEST(Cast, Decimal128ToDecimal128) {
CheckCast(d_28_0, d_38_10_roundtripped, options);
// Precision loss without rescale leads to truncation
- auto d_4_2 = ArrayFromJSON(decimal(4, 2), R"(["12.34"])");
+ auto d_4_2 = ArrayFromJSON(decimal128(4, 2), R"(["12.34"])");
for (auto expected : {
- ArrayFromJSON(decimal(3, 2), R"(["12.34"])"),
- ArrayFromJSON(decimal(4, 3), R"(["12.340"])"),
- ArrayFromJSON(decimal(2, 1), R"(["12.3"])"),
+ ArrayFromJSON(decimal128(3, 2), R"(["12.34"])"),
+ ArrayFromJSON(decimal128(4, 3), R"(["12.340"])"),
+ ArrayFromJSON(decimal128(2, 1), R"(["12.3"])"),
}) {
options.allow_decimal_truncate = true;
- CheckCast(d_4_2, expected, options);
+ ASSERT_OK_AND_ASSIGN(auto invalid, Cast(d_4_2, expected->type(), options));
+ ASSERT_NOT_OK(invalid.make_array()->ValidateFull());
Review comment:
Nit: perhaps use `ASSERT_RAISES(Invalid, ...)`?
##########
File path: cpp/src/arrow/testing/util.cc
##########
@@ -84,8 +84,12 @@ std::string random_string(int64_t n, uint32_t seed) {
void random_decimals(int64_t n, uint32_t seed, int32_t precision, uint8_t*
out) {
pcg32_fast gen(seed);
- std::uniform_int_distribution<uint32_t> d(0,
std::numeric_limits<uint8_t>::max());
- const int32_t required_bytes = DecimalType::DecimalSize(precision);
+ // Generate smaller values than requested to avoid generating invalid
decimals for the
Review comment:
`RandomArrayGenerator` has a more precise random generation routine,
should we simply switch to it?
##########
File path: cpp/src/parquet/arrow/test_util.h
##########
@@ -131,8 +131,12 @@ ::arrow::enable_if_fixed_size_binary<ArrowType, Status>
NonNullArray(
static void random_decimals(int64_t n, uint32_t seed, int32_t precision,
uint8_t* out) {
std::default_random_engine gen(seed);
- std::uniform_int_distribution<uint32_t> d(0,
std::numeric_limits<uint8_t>::max());
- const int32_t required_bytes = ::arrow::DecimalType::DecimalSize(precision);
+ // Generate smaller values than requested to avoid generating invalid
decimals for the
+ // given precision
+ const int32_t required_bytes =
+ std::max(1, ::arrow::DecimalType::DecimalSize(precision) - 1);
+ std::uniform_int_distribution<uint32_t> d(
+ 0, required_bytes <= 1 ? 9 : std::numeric_limits<uint8_t>::max());
Review comment:
Same comment as in the other occurrence, though this may be less trivial
in the Parquet codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
