Skip to content

Commit d4eeabe

Browse files
committed
GH-49817: [Gandiva] Round long strings before decimal conversion
Signed-off-by: 1fanwang <1fannnw@gmail.com>
1 parent 2be5dbb commit d4eeabe

6 files changed

Lines changed: 131 additions & 32 deletions

File tree

‎cpp/src/arrow/util/decimal.cc‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -801,14 +801,6 @@ static inline bool ShiftAndAddWithOverflow(std::string_view input, uint64_t out[
801801

802802
namespace {
803803

804-
struct DecimalComponents {
805-
std::string_view whole_digits;
806-
std::string_view fractional_digits;
807-
int32_t exponent = 0;
808-
char sign = 0;
809-
bool has_exponent = false;
810-
};
811-
812804
inline bool IsSign(char c) { return c == '-' || c == '+'; }
813805

814806
inline bool IsDot(char c) { return c == '.'; }
@@ -829,7 +821,10 @@ inline size_t ParseDigitsRun(const char* s, size_t start, size_t size,
829821
return pos;
830822
}
831823

832-
bool ParseDecimalComponents(const char* s, size_t size, DecimalComponents* out) {
824+
} // namespace
825+
826+
bool internal::ParseDecimalComponents(const char* s, size_t size,
827+
DecimalComponents* out) {
833828
size_t pos = 0;
834829

835830
if (size == 0) {
@@ -871,15 +866,17 @@ bool ParseDecimalComponents(const char* s, size_t size, DecimalComponents* out)
871866
return pos == size;
872867
}
873868

869+
namespace {
870+
874871
template <typename Decimal>
875872
Status DecimalFromString(const char* type_name, std::string_view s, Decimal* out,
876873
int32_t* precision, int32_t* scale) {
877874
if (s.empty()) {
878875
return Status::Invalid("Empty string cannot be converted to ", type_name);
879876
}
880877

881-
DecimalComponents dec;
882-
if (!ParseDecimalComponents(s.data(), s.size(), &dec)) {
878+
internal::DecimalComponents dec;
879+
if (!internal::ParseDecimalComponents(s.data(), s.size(), &dec)) {
883880
return Status::Invalid("The string '", s, "' is not a valid ", type_name, " number");
884881
}
885882

@@ -953,8 +950,8 @@ Status SimpleDecimalFromString(const char* type_name, std::string_view s,
953950
return Status::Invalid("Empty string cannot be converted to ", type_name);
954951
}
955952

956-
DecimalComponents dec;
957-
if (!ParseDecimalComponents(s.data(), s.size(), &dec)) {
953+
internal::DecimalComponents dec;
954+
if (!internal::ParseDecimalComponents(s.data(), s.size(), &dec)) {
958955
return Status::Invalid("The string '", s, "' is not a valid ", type_name, " number");
959956
}
960957

‎cpp/src/arrow/util/decimal_internal.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <array>
2121
#include <cstdint>
2222
#include <limits>
23+
#include <string_view>
2324
#include <type_traits>
2425

2526
#include "arrow/type_fwd.h"
@@ -30,6 +31,21 @@
3031

3132
namespace arrow {
3233

34+
namespace internal {
35+
36+
struct DecimalComponents {
37+
std::string_view whole_digits;
38+
std::string_view fractional_digits;
39+
int32_t exponent = 0;
40+
char sign = 0;
41+
bool has_exponent = false;
42+
};
43+
44+
ARROW_EXPORT bool ParseDecimalComponents(const char* s, size_t size,
45+
DecimalComponents* out);
46+
47+
} // namespace internal
48+
3349
constexpr auto kInt32DecimalDigits =
3450
static_cast<size_t>(std::numeric_limits<int32_t>::digits10);
3551

‎cpp/src/gandiva/gdv_function_stubs.cc‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323
#include <cstdio>
2424
#include <sstream>
2525
#include <string>
26+
#include <string_view>
2627
#include <vector>
2728

2829
#include "arrow/util/base64.h"
2930
#include "arrow/util/bit_util.h"
31+
#include "arrow/util/decimal_internal.h"
3032
#include "arrow/util/double_conversion_internal.h"
3133
#include "arrow/util/value_parsing.h"
3234

@@ -204,11 +206,51 @@ CRC_FUNCTION(utf8)
204206
CRC_FUNCTION(binary)
205207

206208
int32_t gdv_fn_dec_from_string(int64_t context, const char* in, int32_t in_length,
207-
int32_t* precision_from_str, int32_t* scale_from_str,
208-
int64_t* dec_high_from_str, uint64_t* dec_low_from_str) {
209+
int32_t out_scale, int32_t* precision_from_str,
210+
int32_t* scale_from_str, int64_t* dec_high_from_str,
211+
uint64_t* dec_low_from_str) {
209212
arrow::Decimal128 dec;
210-
auto status = arrow::Decimal128::FromString(std::string(in, in_length), &dec,
211-
precision_from_str, scale_from_str);
213+
const std::string_view input(in, in_length);
214+
auto status =
215+
arrow::Decimal128::FromString(input, &dec, precision_from_str, scale_from_str);
216+
if (!status.ok() ||
217+
static_cast<int64_t>(*scale_from_str) - out_scale > arrow::Decimal128::kMaxScale) {
218+
arrow::internal::DecimalComponents components;
219+
if (arrow::internal::ParseDecimalComponents(input.data(), input.size(),
220+
&components)) {
221+
std::string digits(components.whole_digits);
222+
digits.append(components.fractional_digits);
223+
digits.erase(0, digits.find_first_not_of('0'));
224+
const int64_t num_digits =
225+
static_cast<int64_t>(digits.size()) + out_scale -
226+
static_cast<int64_t>(components.fractional_digits.size()) + components.exponent;
227+
bool round_up = false;
228+
if (num_digits < 0 || num_digits > arrow::Decimal128::kMaxPrecision) {
229+
digits = "0";
230+
} else {
231+
round_up = num_digits < static_cast<int64_t>(digits.size()) &&
232+
digits[static_cast<size_t>(num_digits)] >= '5';
233+
digits.resize(static_cast<size_t>(num_digits), '0');
234+
if (digits.empty()) {
235+
digits = "0";
236+
}
237+
}
238+
status =
239+
arrow::Decimal128::FromString(digits, &dec, precision_from_str, scale_from_str);
240+
if (status.ok()) {
241+
if (round_up) {
242+
dec += arrow::Decimal128(1);
243+
if (dec == arrow::Decimal128::GetScaleMultiplier(*precision_from_str)) {
244+
++(*precision_from_str);
245+
}
246+
}
247+
if (components.sign == '-') {
248+
dec.Negate();
249+
}
250+
*scale_from_str = out_scale;
251+
}
252+
}
253+
}
212254
if (!status.ok()) {
213255
gdv_fn_context_set_error_msg(context, status.message().data());
214256
return -1;
@@ -948,6 +990,7 @@ arrow::Status ExportedStubFunctions::AddMappings(Engine* engine) const {
948990
types->i64_type(), // context
949991
types->i8_ptr_type(), // const char* in
950992
types->i32_type(), // int32_t in_length
993+
types->i32_type(), // int32_t out_scale
951994
types->i32_ptr_type(), // int32_t* precision_from_str
952995
types->i32_ptr_type(), // int32_t* scale_from_str
953996
types->i64_ptr_type(), // int64_t* dec_high_from_str

‎cpp/src/gandiva/gdv_function_stubs.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ const char* gdv_fn_sha1_decimal128(int64_t context, int64_t x_high, uint64_t x_l
128128
gdv_boolean x_isvalid, int32_t* out_length);
129129

130130
int32_t gdv_fn_dec_from_string(int64_t context, const char* in, int32_t in_length,
131-
int32_t* precision_from_str, int32_t* scale_from_str,
132-
int64_t* dec_high_from_str, uint64_t* dec_low_from_str);
131+
int32_t out_scale, int32_t* precision_from_str,
132+
int32_t* scale_from_str, int64_t* dec_high_from_str,
133+
uint64_t* dec_low_from_str);
133134

134135
char* gdv_fn_dec_to_string(int64_t context, int64_t x_high, uint64_t x_low,
135136
int32_t x_scale, int32_t* dec_str_len);

‎cpp/src/gandiva/precompiled/decimal_wrapper.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,8 +403,8 @@ void castDECIMAL_utf8(int64_t context, const char* in, int32_t in_length,
403403
int32_t precision_from_str;
404404
int32_t scale_from_str;
405405
int32_t status =
406-
gdv_fn_dec_from_string(context, in, in_length, &precision_from_str, &scale_from_str,
407-
&dec_high_from_str, &dec_low_from_str);
406+
gdv_fn_dec_from_string(context, in, in_length, out_scale, &precision_from_str,
407+
&scale_from_str, &dec_high_from_str, &dec_low_from_str);
408408
if (status != 0) {
409409
*out_high = 0;
410410
*out_low = 0;

‎cpp/src/gandiva/tests/decimal_test.cc‎

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,18 +1083,60 @@ TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInput) {
10831083
// Create a row-batch with some sample data
10841084
int num_records = 5;
10851085

1086-
// invalid input
1087-
auto invalid_in = MakeArrowArrayUtf8({"a10.5134", "-0.0", "-0.1", "10.516", "-1000"},
1088-
{true, false, true, true, true});
1089-
1090-
// prepare input record batch
1091-
auto in_batch_1 = arrow::RecordBatch::Make(schema, num_records, {invalid_in});
1086+
for (const std::string& invalid :
1087+
std::vector<std::string>{"a10.5134", "1." + std::string(100, '5') + "x",
1088+
"1." + std::string(100, '5') + "e2147483648"}) {
1089+
SCOPED_TRACE(invalid);
1090+
auto invalid_in = MakeArrowArrayUtf8({invalid, "-0.0", "-0.1", "10.516", "-1000"},
1091+
{true, false, true, true, true});
1092+
auto in_batch = arrow::RecordBatch::Make(schema, num_records, {invalid_in});
1093+
arrow::ArrayVector outputs;
1094+
status = projector->Evaluate(*in_batch, pool_, &outputs);
1095+
EXPECT_FALSE(status.ok()) << status.message();
1096+
EXPECT_NE(status.message().find("not a valid decimal128 number"), std::string::npos);
1097+
}
1098+
}
10921099

1093-
// Evaluate expression
1094-
arrow::ArrayVector outputs_1;
1095-
status = projector->Evaluate(*in_batch_1, pool_, &outputs_1);
1096-
EXPECT_FALSE(status.ok()) << status.message();
1097-
EXPECT_NE(status.message().find("not a valid decimal128 number"), std::string::npos);
1100+
TEST_F(TestDecimal, TestCastDecimalVarCharLongInputs) {
1101+
struct TestCase {
1102+
std::string input;
1103+
int32_t precision;
1104+
int32_t scale;
1105+
std::string expected;
1106+
};
1107+
for (const auto& test_case : std::vector<TestCase>{
1108+
{"1." + std::string(50, '5'), 38, 37, "1." + std::string(36, '5') + "6"},
1109+
{"-1." + std::string(50, '5'), 38, 37, "-1." + std::string(36, '5') + "6"},
1110+
{"1." + std::string(100, '1'), 38, 37, "1." + std::string(37, '1')},
1111+
{"15." + std::string(50, '5') + "e-1", 38, 37,
1112+
"1." + std::string(36, '5') + "6"},
1113+
{"0." + std::string(37, '0') + "5" + std::string(70, '0'), 38, 37,
1114+
"0." + std::string(36, '0') + "1"},
1115+
{"0." + std::string(100, '0') + "5", 38, 37, "0"},
1116+
{"9." + std::string(80, '9'), 4, 2, "10.00"},
1117+
{"-9." + std::string(80, '9'), 4, 2, "-10.00"},
1118+
{"99." + std::string(80, '9'), 4, 2, "0.00"},
1119+
{"0001." + std::string(80, '0'), 4, 2, "1.00"},
1120+
{"1." + std::string(100, '0') + "e+2", 6, 2, "100.00"},
1121+
{"1234" + std::string(100, '0') + "e-102", 4, 2, "12.34"}}) {
1122+
SCOPED_TRACE(test_case.input);
1123+
auto decimal_type = arrow::decimal128(test_case.precision, test_case.scale);
1124+
auto field_str = field("in_str", utf8());
1125+
auto schema = arrow::schema({field_str});
1126+
auto expr = TreeExprBuilder::MakeExpression("castDECIMAL", {field_str},
1127+
field("out", decimal_type));
1128+
std::shared_ptr<Projector> projector;
1129+
ASSERT_OK(Projector::Make(schema, {expr}, TestConfiguration(), &projector));
1130+
1131+
auto input = MakeArrowArrayUtf8({test_case.input, ""}, {true, false});
1132+
auto batch = arrow::RecordBatch::Make(schema, input->length(), {input});
1133+
arrow::ArrayVector outputs;
1134+
ASSERT_OK(projector->Evaluate(*batch, pool_, &outputs));
1135+
auto expected = MakeArrowArrayDecimal(
1136+
decimal_type, MakeDecimalVector({test_case.expected, "0"}, test_case.scale),
1137+
{true, false});
1138+
EXPECT_ARROW_ARRAY_EQUALS(expected, outputs[0]);
1139+
}
10981140
}
10991141

11001142
TEST_F(TestDecimal, TestVarCharDecimalNestedCast) {

0 commit comments

Comments
 (0)