From 8c28db3fd76ba4f2e8d9dd2d2bcdbaf11570e170 Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Thu, 25 Nov 2021 02:42:50 +0000 Subject: [PATCH 1/3] ARROW-14842: [C++] Improve precision range error messages for Decimal Decimal128: "Invalid: Decimal precision out of range [1, 38]: 0" Decimal256: "Invalid: Decimal precision out of range [1, 76]: 100" --- c_glib/test/test-decimal128-data-type.rb | 2 +- c_glib/test/test-decimal256-data-type.rb | 2 +- cpp/src/arrow/type.cc | 8 ++++++-- r/tests/testthat/test-data-type.R | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/c_glib/test/test-decimal128-data-type.rb b/c_glib/test/test-decimal128-data-type.rb index bcee24187f4..92f2f47f0bd 100644 --- a/c_glib/test/test-decimal128-data-type.rb +++ b/c_glib/test/test-decimal128-data-type.rb @@ -48,7 +48,7 @@ def test_deciaml_data_type_new def test_invalid_precision message = - "[decimal128-data-type][new]: Invalid: Decimal precision out of range: 39" + "[decimal128-data-type][new]: Invalid: Decimal precision out of range [1, 38]: 39" assert_raise(Arrow::Error::Invalid.new(message)) do Arrow::Decimal128DataType.new(39, 1) end diff --git a/c_glib/test/test-decimal256-data-type.rb b/c_glib/test/test-decimal256-data-type.rb index 3070a4e4c6c..b26f7396043 100644 --- a/c_glib/test/test-decimal256-data-type.rb +++ b/c_glib/test/test-decimal256-data-type.rb @@ -48,7 +48,7 @@ def test_deciaml_data_type_new def test_invalid_precision message = - "[decimal256-data-type][new]: Invalid: Decimal precision out of range: 77" + "[decimal256-data-type][new]: Invalid: Decimal precision out of range [1, 76]: 77" assert_raise(Arrow::Error::Invalid.new(message)) do Arrow::Decimal256DataType.new(77, 1) end diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc index bed7536319e..7b761b3a626 100644 --- a/cpp/src/arrow/type.cc +++ b/cpp/src/arrow/type.cc @@ -831,7 +831,9 @@ Decimal128Type::Decimal128Type(int32_t precision, int32_t scale) Result> Decimal128Type::Make(int32_t precision, int32_t scale) { if (precision < kMinPrecision || precision > kMaxPrecision) { - return Status::Invalid("Decimal precision out of range: ", precision); + const auto min = kMinPrecision, max = kMaxPrecision; + return Status::Invalid("Decimal precision out of range [", min, ", ", max, + "]: ", precision); } return std::make_shared(precision, scale); } @@ -847,7 +849,9 @@ Decimal256Type::Decimal256Type(int32_t precision, int32_t scale) Result> Decimal256Type::Make(int32_t precision, int32_t scale) { if (precision < kMinPrecision || precision > kMaxPrecision) { - return Status::Invalid("Decimal precision out of range: ", precision); + const auto min = kMinPrecision, max = kMaxPrecision; + return Status::Invalid("Decimal precision out of range [", min, ", ", max, + "]: ", precision); } return std::make_shared(precision, scale); } diff --git a/r/tests/testthat/test-data-type.R b/r/tests/testthat/test-data-type.R index a9d0879b8a0..3c4ddfa9813 100644 --- a/r/tests/testthat/test-data-type.R +++ b/r/tests/testthat/test-data-type.R @@ -390,8 +390,8 @@ test_that("decimal type and validation", { expect_error(decimal(4)) expect_error(decimal(4, "two"), '"scale" must be an integer') expect_error(decimal(NA, 2), '"precision" must be an integer') - expect_error(decimal(0, 2), "Invalid: Decimal precision out of range: 0") - expect_error(decimal(100, 2), "Invalid: Decimal precision out of range: 100") + expect_error(decimal(0, 2), "Invalid: Decimal precision out of range [1, 38]: 0") + expect_error(decimal(100, 2), "Invalid: Decimal precision out of range [1, 38]: 100") expect_error(decimal(4, NA), '"scale" must be an integer') expect_r6_class(decimal(4, 2), "Decimal128Type") From 113ae08052f2772787eecd516792ef89f7d75d5d Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Thu, 25 Nov 2021 04:43:59 +0000 Subject: [PATCH 2/3] R expect_error with "fixed = TRUE" --- r/tests/testthat/test-data-type.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-data-type.R b/r/tests/testthat/test-data-type.R index 3c4ddfa9813..98dc9ebfbda 100644 --- a/r/tests/testthat/test-data-type.R +++ b/r/tests/testthat/test-data-type.R @@ -390,8 +390,8 @@ test_that("decimal type and validation", { expect_error(decimal(4)) expect_error(decimal(4, "two"), '"scale" must be an integer') expect_error(decimal(NA, 2), '"precision" must be an integer') - expect_error(decimal(0, 2), "Invalid: Decimal precision out of range [1, 38]: 0") - expect_error(decimal(100, 2), "Invalid: Decimal precision out of range [1, 38]: 100") + expect_error(decimal(0, 2), "Invalid: Decimal precision out of range [1, 38]: 0", fixed = TRUE) + expect_error(decimal(100, 2), "Invalid: Decimal precision out of range [1, 38]: 100", fixed = TRUE) expect_error(decimal(4, NA), '"scale" must be an integer') expect_r6_class(decimal(4, 2), "Decimal128Type") From b83b6ab26b72790134ba123d5fffaa6c7a64fdf2 Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Tue, 30 Nov 2021 02:29:41 +0000 Subject: [PATCH 3/3] workaround static constexpr member odr-used issue --- cpp/src/arrow/type.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc index 7b761b3a626..a01ba0532c5 100644 --- a/cpp/src/arrow/type.cc +++ b/cpp/src/arrow/type.cc @@ -831,9 +831,8 @@ Decimal128Type::Decimal128Type(int32_t precision, int32_t scale) Result> Decimal128Type::Make(int32_t precision, int32_t scale) { if (precision < kMinPrecision || precision > kMaxPrecision) { - const auto min = kMinPrecision, max = kMaxPrecision; - return Status::Invalid("Decimal precision out of range [", min, ", ", max, - "]: ", precision); + return Status::Invalid("Decimal precision out of range [", int32_t(kMinPrecision), + ", ", int32_t(kMaxPrecision), "]: ", precision); } return std::make_shared(precision, scale); } @@ -849,9 +848,8 @@ Decimal256Type::Decimal256Type(int32_t precision, int32_t scale) Result> Decimal256Type::Make(int32_t precision, int32_t scale) { if (precision < kMinPrecision || precision > kMaxPrecision) { - const auto min = kMinPrecision, max = kMaxPrecision; - return Status::Invalid("Decimal precision out of range [", min, ", ", max, - "]: ", precision); + return Status::Invalid("Decimal precision out of range [", int32_t(kMinPrecision), + ", ", int32_t(kMaxPrecision), "]: ", precision); } return std::make_shared(precision, scale); }