diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 42c1ec8f040..f5cdec59aed 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -577,11 +577,13 @@ set(ARROW_LINK_LIBS set(ARROW_SHARED_PRIVATE_LINK_LIBS ${BOOST_SYSTEM_LIBRARY} - ${BOOST_FILESYSTEM_LIBRARY}) + ${BOOST_FILESYSTEM_LIBRARY} + ${BOOST_REGEX_LIBRARY}) set(ARROW_STATIC_PRIVATE_LINK_LIBS ${BOOST_SYSTEM_LIBRARY} - ${BOOST_FILESYSTEM_LIBRARY}) + ${BOOST_FILESYSTEM_LIBRARY} + ${BOOST_REGEX_LIBRARY}) if (NOT MSVC) set(ARROW_LINK_LIBS diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 3511d40d42c..22eabde0820 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -157,8 +157,11 @@ if (ARROW_BOOST_VENDORED) "${BOOST_LIB_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}boost_system${CMAKE_STATIC_LIBRARY_SUFFIX}") set(BOOST_STATIC_FILESYSTEM_LIBRARY "${BOOST_LIB_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}boost_filesystem${CMAKE_STATIC_LIBRARY_SUFFIX}") + set(BOOST_STATIC_REGEX_LIBRARY + "${BOOST_LIB_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}boost_regex${CMAKE_STATIC_LIBRARY_SUFFIX}") set(BOOST_SYSTEM_LIBRARY "${BOOST_STATIC_SYSTEM_LIBRARY}") set(BOOST_FILESYSTEM_LIBRARY "${BOOST_STATIC_FILESYSTEM_LIBRARY}") + set(BOOST_REGEX_LIBRARY "${BOOST_STATIC_REGEX_LIBRARY}") if (ARROW_BOOST_HEADER_ONLY) set(BOOST_BUILD_PRODUCTS) set(BOOST_CONFIGURE_COMMAND "") @@ -166,7 +169,8 @@ if (ARROW_BOOST_VENDORED) else() set(BOOST_BUILD_PRODUCTS ${BOOST_SYSTEM_LIBRARY} - ${BOOST_FILESYSTEM_LIBRARY}) + ${BOOST_FILESYSTEM_LIBRARY} + ${BOOST_REGEX_LIBRARY}) set(BOOST_CONFIGURE_COMMAND "./bootstrap.sh" "--prefix=${BOOST_PREFIX}" @@ -210,16 +214,19 @@ else() if (ARROW_BOOST_HEADER_ONLY) find_package(Boost REQUIRED) else() - find_package(Boost COMPONENTS system filesystem REQUIRED) + find_package(Boost COMPONENTS system filesystem regex REQUIRED) if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG") set(BOOST_SHARED_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_DEBUG}) set(BOOST_SHARED_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_DEBUG}) + set(BOOST_SHARED_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_DEBUG}) else() set(BOOST_SHARED_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_RELEASE}) set(BOOST_SHARED_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_RELEASE}) + set(BOOST_SHARED_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_RELEASE}) endif() set(BOOST_SYSTEM_LIBRARY boost_system_shared) set(BOOST_FILESYSTEM_LIBRARY boost_filesystem_shared) + set(BOOST_REGEX_LIBRARY boost_regex_shared) endif() else() # Find static boost headers and libs @@ -228,16 +235,19 @@ else() if (ARROW_BOOST_HEADER_ONLY) find_package(Boost REQUIRED) else() - find_package(Boost COMPONENTS system filesystem REQUIRED) + find_package(Boost COMPONENTS system filesystem regex REQUIRED) if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG") set(BOOST_STATIC_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_DEBUG}) set(BOOST_STATIC_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_DEBUG}) + set(BOOST_STATIC_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_DEBUG}) else() set(BOOST_STATIC_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_RELEASE}) set(BOOST_STATIC_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_RELEASE}) + set(BOOST_STATIC_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_RELEASE}) endif() set(BOOST_SYSTEM_LIBRARY boost_system_static) set(BOOST_FILESYSTEM_LIBRARY boost_filesystem_static) + set(BOOST_REGEX_LIBRARY boost_regex_static) endif() endif() endif() @@ -254,7 +264,11 @@ if (NOT ARROW_BOOST_HEADER_ONLY) STATIC_LIB "${BOOST_STATIC_FILESYSTEM_LIBRARY}" SHARED_LIB "${BOOST_SHARED_FILESYSTEM_LIBRARY}") - SET(ARROW_BOOST_LIBS boost_system boost_filesystem) + ADD_THIRDPARTY_LIB(boost_regex + STATIC_LIB "${BOOST_STATIC_REGEX_LIBRARY}" + SHARED_LIB "${BOOST_SHARED_REGEX_LIBRARY}") + + SET(ARROW_BOOST_LIBS boost_system boost_filesystem boost_regex) endif() include_directories(SYSTEM ${Boost_INCLUDE_DIR}) diff --git a/cpp/src/arrow/python/helpers.cc b/cpp/src/arrow/python/helpers.cc index df1db99911b..75fb7d8c7bb 100644 --- a/cpp/src/arrow/python/helpers.cc +++ b/cpp/src/arrow/python/helpers.cc @@ -99,7 +99,8 @@ Status InferDecimalPrecisionAndScale(PyObject* python_decimal, int32_t* precisio DCHECK_NE(precision, NULLPTR); DCHECK_NE(scale, NULLPTR); - OwnedRef as_tuple(PyObject_CallMethod(python_decimal, "as_tuple", "()")); + // TODO(phillipc): Make sure we perform PyDecimal_Check(python_decimal) as a DCHECK + OwnedRef as_tuple(PyObject_CallMethod(python_decimal, "as_tuple", "")); RETURN_IF_PYERROR(); DCHECK(PyTuple_Check(as_tuple.obj())); @@ -117,7 +118,21 @@ Status InferDecimalPrecisionAndScale(PyObject* python_decimal, int32_t* precisio const auto exponent = static_cast(PyLong_AsLong(py_exponent.obj())); RETURN_IF_PYERROR(); - *precision = num_digits; + const int32_t abs_exponent = std::abs(exponent); + + int32_t num_additional_zeros; + + if (num_digits < abs_exponent) { + DCHECK_NE(exponent, 0) << "exponent should never be zero here"; + + // we have leading/trailing zeros, leading if exponent is negative + num_additional_zeros = exponent < 0 ? abs_exponent - num_digits : exponent; + } else { + // we can use the number of digits as the precision + num_additional_zeros = 0; + } + + *precision = num_digits + num_additional_zeros; *scale = -exponent; return Status::OK(); } diff --git a/cpp/src/arrow/python/numpy_to_arrow.cc b/cpp/src/arrow/python/numpy_to_arrow.cc index 23418ad920c..a7104832eac 100644 --- a/cpp/src/arrow/python/numpy_to_arrow.cc +++ b/cpp/src/arrow/python/numpy_to_arrow.cc @@ -766,10 +766,7 @@ Status NumPyConverter::ConvertDecimals() { RETURN_NOT_OK(internal::InferDecimalPrecisionAndScale(objects[i], &tmp_precision, &tmp_scale)); precision = std::max(precision, tmp_precision); - - if (std::abs(desired_scale) < std::abs(tmp_scale)) { - desired_scale = tmp_scale; - } + desired_scale = std::max(desired_scale, tmp_scale); } type_ = ::arrow::decimal(precision, desired_scale); diff --git a/cpp/src/arrow/python/python-test.cc b/cpp/src/arrow/python/python-test.cc index b76caaecee6..6ffcbb131c4 100644 --- a/cpp/src/arrow/python/python-test.cc +++ b/cpp/src/arrow/python/python-test.cc @@ -145,6 +145,57 @@ TEST_F(DecimalTest, TestInferPrecisionAndNegativeScale) { ASSERT_EQ(expected_scale, scale); } +TEST_F(DecimalTest, TestInferAllLeadingZeros) { + std::string decimal_string("0.001"); + OwnedRef python_decimal(this->CreatePythonDecimal(decimal_string)); + + int32_t precision; + int32_t scale; + + ASSERT_OK( + internal::InferDecimalPrecisionAndScale(python_decimal.obj(), &precision, &scale)); + + const int32_t expected_precision = 3; + const int32_t expected_scale = 3; + + ASSERT_EQ(expected_precision, precision); + ASSERT_EQ(expected_scale, scale); +} + +TEST_F(DecimalTest, TestInferAllLeadingZerosExponentialNotationPositive) { + std::string decimal_string("0.01E5"); + OwnedRef python_decimal(this->CreatePythonDecimal(decimal_string)); + + int32_t precision; + int32_t scale; + + ASSERT_OK( + internal::InferDecimalPrecisionAndScale(python_decimal.obj(), &precision, &scale)); + + const int32_t expected_precision = 4; + const int32_t expected_scale = -3; + + ASSERT_EQ(expected_precision, precision); + ASSERT_EQ(expected_scale, scale); +} + +TEST_F(DecimalTest, TestInferAllLeadingZerosExponentialNotationNegative) { + std::string decimal_string("0.01E3"); + OwnedRef python_decimal(this->CreatePythonDecimal(decimal_string)); + + int32_t precision; + int32_t scale; + + ASSERT_OK( + internal::InferDecimalPrecisionAndScale(python_decimal.obj(), &precision, &scale)); + + const int32_t expected_precision = 1; + const int32_t expected_scale = -1; + + ASSERT_EQ(expected_precision, precision); + ASSERT_EQ(expected_scale, scale); +} + TEST(PandasConversionTest, TestObjectBlockWriteFails) { StringBuilder builder; const char value[] = {'\xf1', '\0'}; @@ -241,5 +292,43 @@ TEST_F(DecimalTest, TestOverflowFails) { decimal_type, &value)); } + +TEST_F(DecimalTest, SimpleInference) { + OwnedRef value(this->CreatePythonDecimal("0.01")); + ASSERT_NE(value.obj(), nullptr); + int32_t precision; + int32_t scale; + ASSERT_OK(internal::InferDecimalPrecisionAndScale(value.obj(), &precision, &scale)); + ASSERT_EQ(2, precision); + ASSERT_EQ(2, scale); +} + + +TEST_F(DecimalTest, TestMixedPrecisionAndScaleSequenceConvert) { + + PyAcquireGIL lock; + MemoryPool* pool = default_memory_pool(); + std::shared_ptr arr; + + OwnedRef list_ref(PyList_New(2)); + PyObject* list = list_ref.obj(); + + ASSERT_NE(list, nullptr); + + PyObject* value1 = this->CreatePythonDecimal("0.01").detach(); + ASSERT_NE(value1, nullptr); + + PyObject* value2 = this->CreatePythonDecimal("0.001").detach(); + ASSERT_NE(value2, nullptr); + + // This steals a reference to each object, so we don't need to decref them later + // just the list + + ASSERT_EQ(PyList_SetItem(list, 0, value1), 0); + ASSERT_EQ(PyList_SetItem(list, 1, value2), 0); + + ASSERT_OK(ConvertPySequence(list, pool, &arr)); +} + } // namespace py } // namespace arrow diff --git a/cpp/src/arrow/util/decimal-test.cc b/cpp/src/arrow/util/decimal-test.cc index e4406747d55..42812bd552d 100644 --- a/cpp/src/arrow/util/decimal-test.cc +++ b/cpp/src/arrow/util/decimal-test.cc @@ -37,7 +37,7 @@ class DecimalTestFixture : public ::testing::Test { TEST_F(DecimalTestFixture, TestToString) { Decimal128 decimal(this->integer_value_); - int scale = 5; + int32_t scale = 5; std::string result = decimal.ToString(scale); ASSERT_EQ(result, this->string_value_); } @@ -45,7 +45,7 @@ TEST_F(DecimalTestFixture, TestToString) { TEST_F(DecimalTestFixture, TestFromString) { Decimal128 expected(this->integer_value_); Decimal128 result; - int precision, scale; + int32_t precision, scale; ASSERT_OK(Decimal128::FromString(this->string_value_, &result, &precision, &scale)); ASSERT_EQ(result, expected); ASSERT_EQ(precision, 8); @@ -55,8 +55,8 @@ TEST_F(DecimalTestFixture, TestFromString) { TEST_F(DecimalTestFixture, TestStringStartingWithPlus) { std::string plus_value("+234.234"); Decimal128 out; - int scale; - int precision; + int32_t scale; + int32_t precision; ASSERT_OK(Decimal128::FromString(plus_value, &out, &precision, &scale)); ASSERT_EQ(234234, out); ASSERT_EQ(6, precision); @@ -67,8 +67,8 @@ TEST_F(DecimalTestFixture, TestStringStartingWithPlus128) { std::string plus_value("+2342394230592.232349023094"); Decimal128 expected_value("2342394230592232349023094"); Decimal128 out; - int scale; - int precision; + int32_t scale; + int32_t precision; ASSERT_OK(Decimal128::FromString(plus_value, &out, &precision, &scale)); ASSERT_EQ(expected_value, out); ASSERT_EQ(25, precision); @@ -90,9 +90,7 @@ TEST(DecimalTest, TestFromDecimalString128) { Decimal128 result; ASSERT_OK(Decimal128::FromString(string_value, &result)); Decimal128 expected(static_cast(-230492239423435324)); - expected *= 100; - expected -= 12; - ASSERT_EQ(result, expected); + ASSERT_EQ(result, expected * 100 - 12); // Sanity check that our number is actually using more than 64 bits ASSERT_NE(result.high_bits(), 0); @@ -194,36 +192,36 @@ TEST(DecimalTest, TestInvalidInputWithLeadingZeros) { TEST(DecimalZerosTest, LeadingZerosNoDecimalPoint) { std::string string_value("0000000"); Decimal128 d; - int precision; - int scale; + int32_t precision; + int32_t scale; ASSERT_OK(Decimal128::FromString(string_value, &d, &precision, &scale)); - ASSERT_EQ(precision, 7); - ASSERT_EQ(scale, 0); - ASSERT_EQ(d, 0); + ASSERT_EQ(0, precision); + ASSERT_EQ(0, scale); + ASSERT_EQ(0, d); } TEST(DecimalZerosTest, LeadingZerosDecimalPoint) { std::string string_value("000.0000"); Decimal128 d; - int precision; - int scale; + int32_t precision; + int32_t scale; ASSERT_OK(Decimal128::FromString(string_value, &d, &precision, &scale)); // We explicitly do not support this for now, otherwise this would be ASSERT_EQ - ASSERT_NE(precision, 7); + ASSERT_EQ(4, precision); - ASSERT_EQ(scale, 4); - ASSERT_EQ(d, 0); + ASSERT_EQ(4, scale); + ASSERT_EQ(0, d); } TEST(DecimalZerosTest, NoLeadingZerosDecimalPoint) { std::string string_value(".00000"); Decimal128 d; - int precision; - int scale; + int32_t precision; + int32_t scale; ASSERT_OK(Decimal128::FromString(string_value, &d, &precision, &scale)); - ASSERT_EQ(precision, 5); - ASSERT_EQ(scale, 5); - ASSERT_EQ(d, 0); + ASSERT_EQ(5, precision); + ASSERT_EQ(5, scale); + ASSERT_EQ(0, d); } template @@ -375,4 +373,14 @@ TEST(Decimal128Test, TestSmallNumberFormat) { ASSERT_EQ(expected, result); } +TEST(Decimal128Test, TestNoDecimalPointExponential) { + Decimal128 value; + int32_t precision; + int32_t scale; + ASSERT_OK(Decimal128::FromString("1E1", &value, &precision, &scale)); + ASSERT_EQ(1, value.low_bits()); + ASSERT_EQ(1, precision); + ASSERT_EQ(-1, scale); +} + } // namespace arrow diff --git a/cpp/src/arrow/util/decimal.cc b/cpp/src/arrow/util/decimal.cc index a3c8cda7654..c4d8dfb088b 100644 --- a/cpp/src/arrow/util/decimal.cc +++ b/cpp/src/arrow/util/decimal.cc @@ -23,6 +23,8 @@ #include #include +#include + #include "arrow/util/bit-util.h" #include "arrow/util/decimal.h" #include "arrow/util/logging.h" @@ -187,8 +189,6 @@ static constexpr int64_t kPowersOfTen[kInt64DecimalDigits + 1] = {1LL, 100000000000000000LL, 1000000000000000000LL}; -static inline bool isdigit(char value) { return std::isdigit(value) != 0; } - static void StringToInteger(const std::string& str, Decimal128* out) { using std::size_t; @@ -212,158 +212,112 @@ static void StringToInteger(const std::string& str, Decimal128* out) { } } -Status Decimal128::FromString(const std::string& s, Decimal128* out, int* precision, - int* scale) { - // Implements this regex: "(\\+?|-?)((0*)(\\d*))(\\.(\\d+))?((E|e)(\\+|-)?\\d+)?"; - if (s.empty()) { - return Status::Invalid("Empty string cannot be converted to decimal"); - } +static const boost::regex DECIMAL_REGEX( + // sign of the number + "(?[-+]?)" - std::string::const_iterator charp = s.cbegin(); - std::string::const_iterator end = s.cend(); + // digits around the decimal point + "(((?\\d+)\\.(?\\d*)|\\.(?\\d+)" + ")" - char first_char = *charp; - bool is_negative = false; - if (first_char == '+' || first_char == '-') { - is_negative = first_char == '-'; - ++charp; - } + // optional exponent + "([eE](?[-+]?\\d+))?" - if (charp == end) { - std::stringstream ss; - ss << "Single character: '" << first_char << "' is not a valid decimal value"; - return Status::Invalid(ss.str()); - } + // otherwise + "|" - std::string::const_iterator numeric_string_start = charp; + // we're just an integer + "(?\\d+)" - DCHECK_LT(charp, end); + // or an integer with an exponent + "(?:[eE](?[-+]?\\d+))?)"); - // skip leading zeros - charp = std::find_if_not(charp, end, [](char value) { return value == '0'; }); +static inline bool is_zero_character(char c) { return c == '0'; } - // all zeros and no decimal point - if (charp == end) { - if (out != NULLPTR) { - *out = 0; - } +Status Decimal128::FromString(const std::string& s, Decimal128* out, int32_t* precision, + int32_t* scale) { + if (s.empty()) { + return Status::Invalid("Empty string cannot be converted to decimal"); + } - // Not sure what other libraries assign precision to for this case (this case of - // a string consisting only of one or more zeros) + // case of all zeros + if (std::all_of(s.cbegin(), s.cend(), is_zero_character)) { if (precision != NULLPTR) { - *precision = static_cast(charp - numeric_string_start); + *precision = 0; } if (scale != NULLPTR) { *scale = 0; } + *out = 0; return Status::OK(); } - std::string::const_iterator whole_part_start = charp; + boost::smatch results; + const bool matches = boost::regex_match(s, results, DECIMAL_REGEX); + + if (!matches) { + std::stringstream ss; + ss << s << " does not match"; + return Status::Invalid(ss.str()); + } - charp = std::find_if_not(charp, end, isdigit); + const std::string sign = results["SIGN"]; + const std::string integer = results["INTEGER"]; - std::string::const_iterator whole_part_end = charp; - std::string whole_part(whole_part_start, whole_part_end); + const std::string left_digits = results["LEFT_DIGITS"]; + const std::string first_right_digits = results["FIRST_RIGHT_DIGITS"]; - if (charp != end && *charp == '.') { - ++charp; + const std::string second_right_digits = results["SECOND_RIGHT_DIGITS"]; - if (charp == end) { - return Status::Invalid( - "Decimal point must be followed by at least one base ten digit. Reached the " - "end of the string."); - } + const std::string first_exp_value = results["FIRST_EXP_VALUE"]; + const std::string second_exp_value = results["SECOND_EXP_VALUE"]; - if (std::isdigit(*charp) == 0) { - std::stringstream ss; - ss << "Decimal point must be followed by a base ten digit. Found '" << *charp - << "'"; - return Status::Invalid(ss.str()); - } + std::string whole_part; + std::string fractional_part; + std::string exponent_value; + + if (!integer.empty()) { + whole_part = integer; + } else if (!left_digits.empty()) { + DCHECK(second_right_digits.empty()) << s << " " << second_right_digits; + whole_part = left_digits; + fractional_part = first_right_digits; } else { - if (charp != end) { - std::stringstream ss; - ss << "Expected base ten digit or decimal point but found '" << *charp - << "' instead."; - return Status::Invalid(ss.str()); - } + DCHECK(first_right_digits.empty()) << s << " " << first_right_digits; + fractional_part = second_right_digits; } - std::string::const_iterator fractional_part_start = charp; - - // The rest must be digits or an exponent - if (charp != end) { - charp = std::find_if_not(charp, end, isdigit); + // skip leading zeros before the decimal point + std::string::const_iterator without_leading_zeros = + std::find_if_not(whole_part.cbegin(), whole_part.cend(), is_zero_character); + whole_part = std::string(without_leading_zeros, whole_part.cend()); - // The while loop has ended before the end of the string which means we've hit a - // character that isn't a base ten digit or "E" for exponent - if (charp != end && *charp != 'E' && *charp != 'e') { - std::stringstream ss; - ss << "Found non base ten digit character '" << *charp - << "' before the end of the string"; - return Status::Invalid(ss.str()); - } + if (!first_exp_value.empty()) { + exponent_value = first_exp_value; + } else { + exponent_value = second_exp_value; } - std::string::const_iterator fractional_part_end = charp; - std::string fractional_part(fractional_part_start, fractional_part_end); - if (precision != NULLPTR) { - *precision = static_cast(whole_part.size() + fractional_part.size()); + *precision = static_cast(whole_part.size() + fractional_part.size()); } - if (charp != end) { - // we must have an exponent, if this aborts then we have somehow not caught this and - // raised a proper error - DCHECK(*charp == 'E' || *charp == 'e'); - - ++charp; - - const char value = *charp; - const bool starts_with_plus_or_minus = value == '+' || value == '-'; - - // we use this to construct the adjusted exponent integer later - std::string::const_iterator digit_start = charp; - - // skip plus or minus - charp += starts_with_plus_or_minus; - - // confirm that the rest of the characters are digits - charp = std::find_if_not(charp, end, isdigit); - - if (charp != end) { - // we have something other than digits here - std::stringstream ss; - ss << "Found non decimal digit exponent value '" << *charp << "'"; - return Status::Invalid(ss.str()); - } - - if (scale != NULLPTR) { - // compute the scale from the adjusted exponent - std::string adjusted_exponent_string(digit_start, end); - DCHECK(std::all_of(adjusted_exponent_string.cbegin() + starts_with_plus_or_minus, - adjusted_exponent_string.cend(), isdigit)) - << "Non decimal digit character found in " << adjusted_exponent_string; - const auto adjusted_exponent = - static_cast(std::stol(adjusted_exponent_string)); - const auto len = static_cast(whole_part.size() + fractional_part.size()); - + if (scale != NULLPTR) { + if (!exponent_value.empty()) { + auto adjusted_exponent = static_cast(std::stol(exponent_value)); + auto len = static_cast(whole_part.size() + fractional_part.size()); *scale = -adjusted_exponent + len - 1; - } - } else { - if (scale != NULLPTR) { - *scale = static_cast(fractional_part.size()); + } else { + *scale = static_cast(fractional_part.size()); } } if (out != NULLPTR) { - // zero out in case we've passed in a previously used value *out = 0; StringToInteger(whole_part + fractional_part, out); - if (is_negative) { + if (sign == "-") { out->Negate(); } } diff --git a/cpp/src/arrow/util/decimal.h b/cpp/src/arrow/util/decimal.h index 1594090a0d3..79a99ba6aba 100644 --- a/cpp/src/arrow/util/decimal.h +++ b/cpp/src/arrow/util/decimal.h @@ -124,7 +124,7 @@ class ARROW_EXPORT Decimal128 { /// \brief Convert a decimal string to an Decimal128 value, optionally including /// precision and scale if they're passed in and not null. static Status FromString(const std::string& s, Decimal128* out, - int* precision = NULLPTR, int* scale = NULLPTR); + int32_t* precision = NULLPTR, int32_t* scale = NULLPTR); /// \brief Convert Decimal128 from one scale to another Status Rescale(int32_t original_scale, int32_t new_scale, Decimal128* out) const; diff --git a/python/pyarrow/tests/test_convert_pandas.py b/python/pyarrow/tests/test_convert_pandas.py index 6e68dd961be..04c57ef9f8a 100644 --- a/python/pyarrow/tests/test_convert_pandas.py +++ b/python/pyarrow/tests/test_convert_pandas.py @@ -1143,6 +1143,16 @@ def test_decimal_fails_with_truncation(self): with pytest.raises(pa.ArrowException): pa.array(data2, type=type2) + def test_decimal_with_different_precisions(self): + data = [ + decimal.Decimal('0.01'), + decimal.Decimal('0.001'), + ] + series = pd.Series(data) + array = pa.array(series) + assert array.to_pylist() == data + assert array.type == pa.decimal128(3, 3) + class TestListTypes(object): """