diff --git a/cpp/src/gandiva/function_registry_string.cc b/cpp/src/gandiva/function_registry_string.cc index eb8416872da1..96e34872328b 100644 --- a/cpp/src/gandiva/function_registry_string.cc +++ b/cpp/src/gandiva/function_registry_string.cc @@ -199,7 +199,7 @@ std::vector GetStringFunctionRegistry() { NativeFunction("castVARCHAR", {"varchar"}, DataTypeVector{decimal128(), int64()}, utf8(), kResultNullIfNull, "castVARCHAR_decimal128_int64", - NativeFunction::kNeedsContext), + NativeFunction::kNeedsContext | NativeFunction::kCanReturnErrors), NativeFunction("crc32", {}, DataTypeVector{utf8()}, int64(), kResultNullIfNull, "gdv_fn_crc_32_utf8", NativeFunction::kNeedsContext), @@ -257,6 +257,12 @@ std::vector GetStringFunctionRegistry() { NativeFunction::kNeedsFunctionHolder | NativeFunction::kCanReturnErrors), + NativeFunction("regexp_extract", {}, DataTypeVector{utf8(), utf8()}, utf8(), + kResultNullIfNull, "gdv_fn_regexp_extract_utf8_utf8", + NativeFunction::kNeedsContext | + NativeFunction::kNeedsFunctionHolder | + NativeFunction::kCanReturnErrors), + NativeFunction("regexp_extract", {}, DataTypeVector{utf8(), utf8(), int32()}, utf8(), kResultNullIfNull, "gdv_fn_regexp_extract_utf8_utf8_int32", NativeFunction::kNeedsContext | diff --git a/cpp/src/gandiva/gdv_function_stubs.cc b/cpp/src/gandiva/gdv_function_stubs.cc index cc5e09284d85..25d02096ff37 100644 --- a/cpp/src/gandiva/gdv_function_stubs.cc +++ b/cpp/src/gandiva/gdv_function_stubs.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -192,7 +193,10 @@ int32_t gdv_fn_populate_varlen_vector(int64_t context_ptr, int8_t* data_ptr, GANDIVA_EXPORT \ int64_t gdv_fn_crc_32_##TYPE(int64_t ctx, const char* input, int32_t input_len) { \ if (input_len < 0) { \ - gdv_fn_context_set_error_msg(ctx, "Input length can't be negative"); \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "CRC32: Input length can't be negative, got %d", input_len); \ + gdv_fn_context_set_error_msg(ctx, err_msg); \ return 0; \ } \ boost::crc_32_type result; \ @@ -221,14 +225,19 @@ char* gdv_fn_dec_to_string(int64_t context, int64_t x_high, uint64_t x_low, int32_t x_scale, int32_t* dec_str_len) { arrow::Decimal128 dec(arrow::BasicDecimal128(x_high, x_low)); std::string dec_str = dec.ToString(x_scale); - *dec_str_len = static_cast(dec_str.length()); - char* ret = reinterpret_cast(gdv_fn_context_arena_malloc(context, *dec_str_len)); + auto dec_str_length = static_cast(dec_str.length()); + char* ret = + reinterpret_cast(gdv_fn_context_arena_malloc(context, dec_str_length)); if (ret == nullptr) { std::string err_msg = "Could not allocate memory for string: " + dec_str; gdv_fn_context_set_error_msg(context, err_msg.data()); + // Report zero length so a caller can never combine a positive length with the + // null buffer (the original bug: memcpy(dst, nullptr, positive_len) -> SIGSEGV). + *dec_str_len = 0; return nullptr; } - memcpy(ret, dec_str.data(), *dec_str_len); + *dec_str_len = dec_str_length; + memcpy(ret, dec_str.data(), dec_str_length); return ret; } @@ -236,7 +245,10 @@ GANDIVA_EXPORT const char* gdv_fn_base64_encode_binary(int64_t context, const char* in, int32_t in_len, int32_t* out_len) { if (in_len < 0) { - gdv_fn_context_set_error_msg(context, "Buffer length cannot be negative"); + char err_msg[96]; + snprintf(err_msg, sizeof(err_msg), + "BASE64: input length must be non-negative, got %d", in_len); + gdv_fn_context_set_error_msg(context, err_msg); *out_len = 0; return ""; } @@ -263,7 +275,10 @@ GANDIVA_EXPORT const char* gdv_fn_base64_decode_utf8(int64_t context, const char* in, int32_t in_len, int32_t* out_len) { if (in_len < 0) { - gdv_fn_context_set_error_msg(context, "Buffer length cannot be negative"); + char err_msg[96]; + snprintf(err_msg, sizeof(err_msg), + "UNBASE64: input length must be non-negative, got %d", in_len); + gdv_fn_context_set_error_msg(context, err_msg); *out_len = 0; return ""; } diff --git a/cpp/src/gandiva/gdv_function_stubs_test.cc b/cpp/src/gandiva/gdv_function_stubs_test.cc index d6d459f62bd5..80a4e0adb432 100644 --- a/cpp/src/gandiva/gdv_function_stubs_test.cc +++ b/cpp/src/gandiva/gdv_function_stubs_test.cc @@ -127,7 +127,8 @@ TEST(TestGdvFnStubs, TestBase64Encode) { value = gdv_fn_base64_encode_binary(ctx_ptr, "test", -5, &out_len); out_value = std::string(value, out_len); EXPECT_EQ(out_value, ""); - EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("Buffer length cannot be negative")); + EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("BASE64")); + EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("non-negative")); ctx.Reset(); } @@ -156,7 +157,8 @@ TEST(TestGdvFnStubs, TestBase64Decode) { value = gdv_fn_base64_decode_utf8(ctx_ptr, "test", -5, &out_len); out_value = std::string(value, out_len); EXPECT_EQ(out_value, ""); - EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("Buffer length cannot be negative")); + EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("UNBASE64")); + EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("non-negative")); ctx.Reset(); } diff --git a/cpp/src/gandiva/gdv_string_function_stubs.cc b/cpp/src/gandiva/gdv_string_function_stubs.cc index d271834fb478..3455113fc4c1 100644 --- a/cpp/src/gandiva/gdv_string_function_stubs.cc +++ b/cpp/src/gandiva/gdv_string_function_stubs.cc @@ -70,6 +70,18 @@ const char* gdv_fn_regexp_replace_utf8_utf8( out_length); } +GANDIVA_EXPORT +const char* gdv_fn_regexp_extract_utf8_utf8(int64_t ptr, int64_t holder_ptr, + const char* data, int32_t data_len, + const char* /*pattern*/, + int32_t /*pattern_len*/, + int32_t* out_length) { + gandiva::ExecutionContext* context = reinterpret_cast(ptr); + gandiva::ExtractHolder* holder = reinterpret_cast(holder_ptr); + return (*holder)(context, data, data_len, 1, out_length); +} + +GANDIVA_EXPORT const char* gdv_fn_regexp_extract_utf8_utf8_int32(int64_t ptr, int64_t holder_ptr, const char* data, int32_t data_len, const char* /*pattern*/, @@ -855,6 +867,19 @@ arrow::Status ExportedStringFunctions::AddMappings(Engine* engine) const { "gdv_fn_regexp_extract_utf8_utf8_int32", types->i8_ptr_type() /*return_type*/, args, reinterpret_cast(gdv_fn_regexp_extract_utf8_utf8_int32)); + // gdv_fn_regexp_extract_utf8_utf8 + args = {types->i64_type(), // int64_t ptr + types->i64_type(), // int64_t holder_ptr + types->i8_ptr_type(), // const char* data + types->i32_type(), // int data_len + types->i8_ptr_type(), // const char* pattern + types->i32_type(), // int pattern_len + types->i32_ptr_type()}; // int32_t* out_length + + engine->AddGlobalMappingForFunc( + "gdv_fn_regexp_extract_utf8_utf8", types->i8_ptr_type() /*return_type*/, args, + reinterpret_cast(gdv_fn_regexp_extract_utf8_utf8)); + // gdv_fn_castVARCHAR_int32_int64 args = {types->i64_type(), // int64_t execution_context types->i32_type(), // int32_t value diff --git a/cpp/src/gandiva/precompiled/arithmetic_ops.cc b/cpp/src/gandiva/precompiled/arithmetic_ops.cc index 2bd31bd78870..bebeb191a0f2 100644 --- a/cpp/src/gandiva/precompiled/arithmetic_ops.cc +++ b/cpp/src/gandiva/precompiled/arithmetic_ops.cc @@ -15,8 +15,10 @@ // specific language governing permissions and limitations // under the License. +#include #include #include +#include #include "arrow/util/basic_decimal.h" extern "C" { @@ -65,7 +67,7 @@ extern "C" { gdv_##OUT_TYPE NAME##_##IN_TYPE1##_##IN_TYPE2(int64_t context, gdv_##IN_TYPE1 left, \ gdv_##IN_TYPE2 right) { \ if (right == static_cast(0)) { \ - gdv_fn_context_set_error_msg(context, "divide by zero error"); \ + gdv_fn_context_set_error_msg(context, "PMOD: divide by zero error"); \ return static_cast(0); \ } \ double mod = fmod(static_cast(left), static_cast(right)); \ @@ -109,7 +111,8 @@ PMOD_OP(pmod, float64, float64, float64) gdv_float64 mod_float64_float64(int64_t context, gdv_float64 x, gdv_float64 y) { if (y == 0.0) { - const char* err_msg = "divide by zero error"; + char err_msg[96]; + snprintf(err_msg, sizeof(err_msg), "MOD: divide by zero error (dividend: %g)", x); gdv_fn_context_set_error_msg(context, err_msg); return 0.0; } @@ -351,7 +354,7 @@ NUMERIC_BOOL_DATE_FUNCTION(IS_NOT_DISTINCT_FROM) FORCE_INLINE \ gdv_##TYPE divide_##TYPE##_##TYPE(gdv_int64 context, gdv_##TYPE in1, gdv_##TYPE in2) { \ if (in2 == 0) { \ - const char* err_msg = "divide by zero error"; \ + const char* err_msg = "DIVIDE: divide by zero error"; \ gdv_fn_context_set_error_msg(context, err_msg); \ return 0; \ } \ @@ -376,14 +379,19 @@ NUMERIC_FUNCTION(POSITIVE) NUMERIC_FUNCTION_FOR_REAL(NEGATIVE) -#define NEGATIVE_INTEGER(TYPE, SIZE) \ - FORCE_INLINE \ - gdv_##TYPE negative_##TYPE(gdv_int64 context, gdv_##TYPE in) { \ - if (in <= INT##SIZE##_MIN) { \ - gdv_fn_context_set_error_msg(context, "Overflow in negative execution"); \ - return 0; \ - } \ - return -1 * in; \ +#define NEGATIVE_INTEGER(TYPE, SIZE) \ + FORCE_INLINE \ + gdv_##TYPE negative_##TYPE(gdv_int64 context, gdv_##TYPE in) { \ + if (in <= INT##SIZE##_MIN) { \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "NEGATIVE: Overflow in negative execution " \ + "(cannot negate INT" #SIZE "_MIN: %" PRId64 ")", \ + static_cast(in)); \ + gdv_fn_context_set_error_msg(context, err_msg); \ + return 0; \ + } \ + return -1 * in; \ } NEGATIVE_INTEGER(int32, 32) @@ -396,8 +404,12 @@ const int64_t INT_MIN_TO_NEGATIVE_INTERVAL_DAY_TIME = -9223372030412324863; gdv_int64 negative_daytimeinterval(gdv_int64 context, gdv_day_time_interval interval) { if (interval > INT_MAX_TO_NEGATIVE_INTERVAL_DAY_TIME || interval < INT_MIN_TO_NEGATIVE_INTERVAL_DAY_TIME) { - gdv_fn_context_set_error_msg( - context, "Interval day time is out of boundaries for the negative function"); + char err_msg[128]; + snprintf(err_msg, sizeof(err_msg), + "NEGATIVE: Interval day time is out of boundaries for the negative " + "function (value: %" PRId64 ")", + static_cast(interval)); + gdv_fn_context_set_error_msg(context, err_msg); return 0; } @@ -430,7 +442,7 @@ void negative_decimal(gdv_int64 context, int64_t high_bits, uint64_t low_bits, FORCE_INLINE \ gdv_##TYPE div_##TYPE##_##TYPE(gdv_int64 context, gdv_##TYPE in1, gdv_##TYPE in2) { \ if (in2 == 0) { \ - const char* err_msg = "divide by zero error"; \ + const char* err_msg = "DIV: divide by zero error"; \ gdv_fn_context_set_error_msg(context, err_msg); \ return 0; \ } \ @@ -448,7 +460,7 @@ DIV(uint64) FORCE_INLINE \ gdv_##TYPE div_##TYPE##_##TYPE(gdv_int64 context, gdv_##TYPE in1, gdv_##TYPE in2) { \ if (in2 == 0) { \ - const char* err_msg = "divide by zero error"; \ + const char* err_msg = "DIV: divide by zero error"; \ gdv_fn_context_set_error_msg(context, err_msg); \ return 0; \ } \ diff --git a/cpp/src/gandiva/precompiled/arithmetic_ops_test.cc b/cpp/src/gandiva/precompiled/arithmetic_ops_test.cc index 02fc68713b6b..8753e36faf4f 100644 --- a/cpp/src/gandiva/precompiled/arithmetic_ops_test.cc +++ b/cpp/src/gandiva/precompiled/arithmetic_ops_test.cc @@ -52,7 +52,7 @@ TEST(TestArithmeticOps, TestPmod) { EXPECT_EQ(pmod_int64_int64(ctx, 3, 0), 0); EXPECT_TRUE(context.has_error()); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); context.Reset(); } @@ -65,7 +65,7 @@ TEST(TestArithmeticOps, TestMod) { EXPECT_DOUBLE_EQ(mod_float64_float64(reinterpret_cast(&context), 2.5, 0.0), 0.0); EXPECT_TRUE(context.has_error()); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); context.Reset(); EXPECT_NEAR(mod_float64_float64(reinterpret_cast(&context), 2.5, 1.2), 0.1, @@ -219,8 +219,9 @@ TEST(TestArithmeticOps, TestNegativeIntervalTypes) { result = negative_daytimeinterval(ctx_ptr, INT64_MAX); EXPECT_EQ(ctx.has_error(), true); - EXPECT_EQ(ctx.get_error(), - "Interval day time is out of boundaries for the negative function"); + EXPECT_THAT(ctx.get_error(), + ::testing::HasSubstr( + "Interval day time is out of boundaries for the negative function")); ctx.Reset(); const int64_t INT_MIN_TO_NEGATIVE_INTERVAL_DAY_TIME = -9223372030412324863; @@ -229,8 +230,9 @@ TEST(TestArithmeticOps, TestNegativeIntervalTypes) { result = negative_daytimeinterval(ctx_ptr, INT64_MIN); EXPECT_EQ(ctx.has_error(), true); - EXPECT_EQ(ctx.get_error(), - "Interval day time is out of boundaries for the negative function"); + EXPECT_THAT(ctx.get_error(), + ::testing::HasSubstr( + "Interval day time is out of boundaries for the negative function")); ctx.Reset(); // Month interval @@ -251,7 +253,7 @@ TEST(TestArithmeticOps, TestDivide) { gandiva::ExecutionContext context; EXPECT_EQ(divide_int64_int64(reinterpret_cast(&context), 10, 0), 0); EXPECT_EQ(context.has_error(), true); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); context.Reset(); EXPECT_EQ(divide_int64_int64(reinterpret_cast(&context), 10, 2), 5); @@ -262,7 +264,7 @@ TEST(TestArithmeticOps, TestDiv) { gandiva::ExecutionContext context; EXPECT_EQ(div_int64_int64(reinterpret_cast(&context), 101, 0), 0); EXPECT_EQ(context.has_error(), true); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); context.Reset(); EXPECT_EQ(div_int64_int64(reinterpret_cast(&context), 101, 111), 0); @@ -278,7 +280,7 @@ TEST(TestArithmeticOps, TestDiv) { div_float64_float64(reinterpret_cast(&context), 1010.1010, 0.00000), 0.0); EXPECT_EQ(context.has_error(), true); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); context.Reset(); EXPECT_EQ(div_float32_float32(reinterpret_cast(&context), 1010.1010f, 2.1f), diff --git a/cpp/src/gandiva/precompiled/decimal_ops.cc b/cpp/src/gandiva/precompiled/decimal_ops.cc index 68949680b7c0..9332fbfaa7cb 100644 --- a/cpp/src/gandiva/precompiled/decimal_ops.cc +++ b/cpp/src/gandiva/precompiled/decimal_ops.cc @@ -351,7 +351,7 @@ BasicDecimal128 Divide(int64_t context, const BasicDecimalScalar128& x, const BasicDecimalScalar128& y, int32_t out_precision, int32_t out_scale, bool* overflow) { if (y.value() == 0) { - const char* err_msg = "divide by zero error"; + const char* err_msg = "DIVIDE: divide by zero error (decimal)"; gdv_fn_context_set_error_msg(context, err_msg); return 0; } @@ -396,7 +396,7 @@ BasicDecimal128 Mod(int64_t context, const BasicDecimalScalar128& x, const BasicDecimalScalar128& y, int32_t out_precision, int32_t out_scale, bool* overflow) { if (y.value() == 0) { - const char* err_msg = "divide by zero error"; + const char* err_msg = "MOD: divide by zero error (decimal)"; gdv_fn_context_set_error_msg(context, err_msg); return 0; } diff --git a/cpp/src/gandiva/precompiled/decimal_ops_test.cc b/cpp/src/gandiva/precompiled/decimal_ops_test.cc index be8a1fe8ade4..95b0a874b94f 100644 --- a/cpp/src/gandiva/precompiled/decimal_ops_test.cc +++ b/cpp/src/gandiva/precompiled/decimal_ops_test.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include #include @@ -455,7 +456,7 @@ TEST_F(TestDecimalSql, DivideByZero) { DecimalScalar128{"201", 20, 3}, DecimalScalar128{"0", 20, 2}, result_precision, result_scale, &overflow); EXPECT_TRUE(context.has_error()); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); // divide-by-nonzero should not cause an error. context.Reset(); @@ -472,7 +473,7 @@ TEST_F(TestDecimalSql, DivideByZero) { DecimalScalar128{"0", 20, 2}, result_precision, result_scale, &overflow); EXPECT_TRUE(context.has_error()); - EXPECT_EQ(context.get_error(), "divide by zero error"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")); // mod-by-nonzero should not cause an error. context.Reset(); diff --git a/cpp/src/gandiva/precompiled/decimal_wrapper.cc b/cpp/src/gandiva/precompiled/decimal_wrapper.cc index cffb7ae9781f..f232f35e4e29 100644 --- a/cpp/src/gandiva/precompiled/decimal_wrapper.cc +++ b/cpp/src/gandiva/precompiled/decimal_wrapper.cc @@ -423,11 +423,23 @@ FORCE_INLINE char* castVARCHAR_decimal128_int64(int64_t context, int64_t x_high, uint64_t x_low, int32_t x_precision, int32_t x_scale, int64_t out_len_param, int32_t* out_length) { + if (out_len_param < 0) { + gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); + *out_length = 0; + return const_cast(""); + } int32_t full_dec_str_len; char* dec_str = gdv_fn_dec_to_string(context, x_high, x_low, x_scale, &full_dec_str_len); - int32_t trunc_dec_str_len = - out_len_param < full_dec_str_len ? out_len_param : full_dec_str_len; + if (dec_str == nullptr) { + // Allocation failed upstream; error message is already set. Avoid copying from + // an invalid buffer with a non-zero length. + *out_length = 0; + return const_cast(""); + } + int32_t trunc_dec_str_len = out_len_param < full_dec_str_len + ? static_cast(out_len_param) + : full_dec_str_len; *out_length = trunc_dec_str_len; return dec_str; } diff --git a/cpp/src/gandiva/precompiled/extended_math_ops.cc b/cpp/src/gandiva/precompiled/extended_math_ops.cc index c29f8f2a8684..348d595faf49 100644 --- a/cpp/src/gandiva/precompiled/extended_math_ops.cc +++ b/cpp/src/gandiva/precompiled/extended_math_ops.cc @@ -24,6 +24,7 @@ extern "C" { +#include #include #include #include @@ -80,7 +81,7 @@ ENUMERIC_TYPES_UNARY(LOG10, float64) FORCE_INLINE void set_error_for_logbase(int64_t execution_context, double base) { - const char* prefix = "divide by zero error with log of base"; + const char* prefix = "LOG: divide by zero error with log of base"; int size = static_cast(strlen(prefix)) + 64; char* error = reinterpret_cast(malloc(size)); snprintf(error, size, "%s %f", prefix, base); @@ -251,20 +252,28 @@ static const int64_t kFactorialLookupTable[] = {1, 121645100408832000, 2432902008176640000}; -#define FACTORIAL(IN_TYPE) \ - FORCE_INLINE \ - gdv_int64 factorial_##IN_TYPE(gdv_int64 ctx, gdv_##IN_TYPE value) { \ - if (value < 0) { \ - gdv_fn_context_set_error_msg(ctx, "Factorial of negative number not exist!"); \ - return 0; \ - } \ - /* For numbers greater than 20 causes an overflow. */ \ - if (value > 20) { \ - gdv_fn_context_set_error_msg(ctx, "Numbers greater than 20 cause overflow!"); \ - return 0; \ - } \ - \ - return kFactorialLookupTable[static_cast(value)]; \ +#define FACTORIAL(IN_TYPE) \ + FORCE_INLINE \ + gdv_int64 factorial_##IN_TYPE(gdv_int64 ctx, gdv_##IN_TYPE value) { \ + if (value < 0) { \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "FACTORIAL: input must be non-negative, got %" PRId64, \ + static_cast(value)); \ + gdv_fn_context_set_error_msg(ctx, err_msg); \ + return 0; \ + } \ + /* For numbers greater than 20 causes an overflow. */ \ + if (value > 20) { \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "FACTORIAL: input %" PRId64 " exceeds maximum 20 (would overflow int64)", \ + static_cast(value)); \ + gdv_fn_context_set_error_msg(ctx, err_msg); \ + return 0; \ + } \ + \ + return kFactorialLookupTable[static_cast(value)]; \ } FACTORIAL(int32) diff --git a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc index ad0cb78188c5..e7a0f74c1b45 100644 --- a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc +++ b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc @@ -19,6 +19,7 @@ # define M_PI 3.14159265358979323846 #endif +#include #include #include @@ -61,11 +62,12 @@ TEST(TestExtendedMathOps, TestFactorial) { } factorial_int32(ctx, 21); - EXPECT_TRUE(context.get_error().find("overflow") != std::string::npos); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("overflow")); context.Reset(); factorial_int32(ctx, -5); - EXPECT_TRUE(context.get_error().find("Factorial of negative") != std::string::npos); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("FACTORIAL")); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("non-negative")); context.Reset(); for (int64_t i = 0; i <= 20; ++i) { @@ -79,11 +81,12 @@ TEST(TestExtendedMathOps, TestFactorial) { } factorial_int64(ctx, 21); - EXPECT_TRUE(context.get_error().find("overflow") != std::string::npos); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("overflow")); context.Reset(); factorial_int64(ctx, -5); - EXPECT_TRUE(context.get_error().find("Factorial of negative") != std::string::npos); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("FACTORIAL")); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("non-negative")); context.Reset(); } @@ -125,7 +128,7 @@ TEST(TestExtendedMathOps, TestLogWithBase) { log_int32_int32(reinterpret_cast(&context), 1 /*base*/, 10 /*value*/); VerifyFuzzyEquals(out, 0); EXPECT_EQ(context.has_error(), true); - EXPECT_TRUE(context.get_error().find("divide by zero error") != std::string::npos) + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error")) << context.get_error(); gandiva::ExecutionContext context1; diff --git a/cpp/src/gandiva/precompiled/string_ops.cc b/cpp/src/gandiva/precompiled/string_ops.cc index 035d3c8c62e1..c79a4f355297 100644 --- a/cpp/src/gandiva/precompiled/string_ops.cc +++ b/cpp/src/gandiva/precompiled/string_ops.cc @@ -27,6 +27,7 @@ extern "C" { #include #include #include +#include #include "./types.h" @@ -540,7 +541,9 @@ gdv_boolean compare_lower_strings(const char* base_str, gdv_int32 base_str_len, FORCE_INLINE gdv_boolean castBIT_utf8(gdv_int64 context, const char* data, gdv_int32 data_len) { if (data_len <= 0) { - gdv_fn_context_set_error_msg(context, "Invalid value for boolean."); + gdv_fn_context_set_error_msg(context, + "CAST_BIT: Invalid value for boolean: empty string " + "(expected 0, 1, true, false; case-insensitive)"); return false; } @@ -569,7 +572,10 @@ gdv_boolean castBIT_utf8(gdv_int64 context, const char* data, gdv_int32 data_len if (compare_lower_strings("false", 5, trimmed_data, trimmed_len)) return false; } // if no 'true', 'false', '0' or '1' value is found, set an error - gdv_fn_context_set_error_msg(context, "Invalid value for boolean."); + std::string err_msg = "CAST_BIT: Invalid value for boolean: '" + + std::string(data, data_len) + + "' (expected 0, 1, true, false; case-insensitive)"; + gdv_fn_context_set_error_msg(context, err_msg.c_str()); return false; } @@ -578,7 +584,10 @@ const char* castVARCHAR_bool_int64(gdv_int64 context, gdv_boolean value, gdv_int64 out_len, gdv_int32* out_length) { gdv_int32 len = static_cast(out_len); if (len < 0) { - gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); + char err_msg[96]; + snprintf(err_msg, sizeof(err_msg), + "CAST_VARCHAR: Output buffer length can't be negative, got %d", len); + gdv_fn_context_set_error_msg(context, err_msg); *out_length = 0; return ""; } @@ -592,90 +601,93 @@ const char* castVARCHAR_bool_int64(gdv_int64 context, gdv_boolean value, } // Truncates the string to given length -#define CAST_VARCHAR_FROM_VARLEN_TYPE(TYPE) \ - FORCE_INLINE \ - const char* castVARCHAR_##TYPE##_int64(gdv_int64 context, const char* data, \ - gdv_int32 data_len, int64_t out_len, \ - int32_t* out_length) { \ - int32_t len = static_cast(out_len); \ - \ - if (len < 0) { \ - gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \ - *out_length = 0; \ - return ""; \ - } \ - \ - if (len >= data_len || len == 0) { \ - *out_length = data_len; \ - return data; \ - } \ - \ - int32_t remaining = len; \ - int32_t index = 0; \ - bool is_multibyte = false; \ - do { \ - /* In utf8, MSB of a single byte unicode char is always 0, \ - * whereas for a multibyte character the MSB of each byte is 1. \ - * So for a single byte char, a bitwise-and with x80 (10000000) will be 0 \ - * and it won't be 0 for bytes of a multibyte char. \ - */ \ - char* data_ptr = const_cast(data); \ - \ - /* advance byte by byte till the 8-byte boundary then advance 8 bytes */ \ - auto num_bytes = reinterpret_cast(data_ptr) & 0x07; \ - num_bytes = (8 - num_bytes) & 0x07; \ - while (num_bytes > 0) { \ - uint8_t* ptr = reinterpret_cast(data_ptr + index); \ - if ((*ptr & 0x80) != 0) { \ - is_multibyte = true; \ - break; \ - } \ - index++; \ - remaining--; \ - num_bytes--; \ - } \ - if (is_multibyte) break; \ - while (remaining >= 8) { \ - uint64_t* ptr = reinterpret_cast(data_ptr + index); \ - if ((*ptr & 0x8080808080808080) != 0) { \ - is_multibyte = true; \ - break; \ - } \ - index += 8; \ - remaining -= 8; \ - } \ - if (is_multibyte) break; \ - if (remaining >= 4) { \ - uint32_t* ptr = reinterpret_cast(data_ptr + index); \ - if ((*ptr & 0x80808080) != 0) break; \ - index += 4; \ - remaining -= 4; \ - } \ - while (remaining > 0) { \ - uint8_t* ptr = reinterpret_cast(data_ptr + index); \ - if ((*ptr & 0x80) != 0) { \ - is_multibyte = true; \ - break; \ - } \ - index++; \ - remaining--; \ - } \ - if (is_multibyte) break; \ - /* reached here; all are single byte characters */ \ - *out_length = len; \ - return data; \ - } while (false); \ - \ - /* detected multibyte utf8 characters; slow path */ \ - int32_t byte_pos = \ - utf8_byte_pos(context, data + index, data_len - index, len - index); \ - if (byte_pos < 0) { \ - *out_length = 0; \ - return ""; \ - } \ - \ - *out_length = index + byte_pos; \ - return data; \ +#define CAST_VARCHAR_FROM_VARLEN_TYPE(TYPE) \ + FORCE_INLINE \ + const char* castVARCHAR_##TYPE##_int64(gdv_int64 context, const char* data, \ + gdv_int32 data_len, int64_t out_len, \ + int32_t* out_length) { \ + int32_t len = static_cast(out_len); \ + \ + if (len < 0) { \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "CAST_VARCHAR: Output buffer length can't be negative, got %d", len); \ + gdv_fn_context_set_error_msg(context, err_msg); \ + *out_length = 0; \ + return ""; \ + } \ + \ + if (len >= data_len || len == 0) { \ + *out_length = data_len; \ + return data; \ + } \ + \ + int32_t remaining = len; \ + int32_t index = 0; \ + bool is_multibyte = false; \ + do { \ + /* In utf8, MSB of a single byte unicode char is always 0, \ + * whereas for a multibyte character the MSB of each byte is 1. \ + * So for a single byte char, a bitwise-and with x80 (10000000) will be 0 \ + * and it won't be 0 for bytes of a multibyte char. \ + */ \ + char* data_ptr = const_cast(data); \ + \ + /* advance byte by byte till the 8-byte boundary then advance 8 bytes */ \ + auto num_bytes = reinterpret_cast(data_ptr) & 0x07; \ + num_bytes = (8 - num_bytes) & 0x07; \ + while (num_bytes > 0) { \ + uint8_t* ptr = reinterpret_cast(data_ptr + index); \ + if ((*ptr & 0x80) != 0) { \ + is_multibyte = true; \ + break; \ + } \ + index++; \ + remaining--; \ + num_bytes--; \ + } \ + if (is_multibyte) break; \ + while (remaining >= 8) { \ + uint64_t* ptr = reinterpret_cast(data_ptr + index); \ + if ((*ptr & 0x8080808080808080) != 0) { \ + is_multibyte = true; \ + break; \ + } \ + index += 8; \ + remaining -= 8; \ + } \ + if (is_multibyte) break; \ + if (remaining >= 4) { \ + uint32_t* ptr = reinterpret_cast(data_ptr + index); \ + if ((*ptr & 0x80808080) != 0) break; \ + index += 4; \ + remaining -= 4; \ + } \ + while (remaining > 0) { \ + uint8_t* ptr = reinterpret_cast(data_ptr + index); \ + if ((*ptr & 0x80) != 0) { \ + is_multibyte = true; \ + break; \ + } \ + index++; \ + remaining--; \ + } \ + if (is_multibyte) break; \ + /* reached here; all are single byte characters */ \ + *out_length = len; \ + return data; \ + } while (false); \ + \ + /* detected multibyte utf8 characters; slow path */ \ + int32_t byte_pos = \ + utf8_byte_pos(context, data + index, data_len - index, len - index); \ + if (byte_pos < 0) { \ + *out_length = 0; \ + return ""; \ + } \ + \ + *out_length = index + byte_pos; \ + return data; \ } CAST_VARCHAR_FROM_VARLEN_TYPE(utf8) @@ -691,7 +703,10 @@ CAST_VARCHAR_FROM_VARLEN_TYPE(binary) int32_t* out_length) { \ int32_t len = static_cast(out_len); \ if (len < 0) { \ - gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "CAST_VARBINARY: Output buffer length can't be negative, got %d", len); \ + gdv_fn_context_set_error_msg(context, err_msg); \ *out_length = 0; \ return ""; \ } \ @@ -839,13 +854,21 @@ const char* repeat_utf8_int32(gdv_int64 context, const char* in, gdv_int32 in_le } // if the repeat number is a negative number, an error is set on context if (repeat_number < 0) { - gdv_fn_context_set_error_msg(context, "Repeat number can't be negative"); + char err_msg[96]; + snprintf(err_msg, sizeof(err_msg), "REPEAT: Repeat number can't be negative, got %d", + repeat_number); + gdv_fn_context_set_error_msg(context, err_msg); *out_len = 0; return ""; } if (ARROW_PREDICT_FALSE( arrow::internal::MultiplyWithOverflow(repeat_number, in_len, out_len))) { - gdv_fn_context_set_error_msg(context, "Would overflow maximum output size"); + char err_msg[128]; + snprintf(err_msg, sizeof(err_msg), + "REPEAT: Would overflow maximum output size " + "(repeat count %d * input length %d)", + repeat_number, in_len); + gdv_fn_context_set_error_msg(context, err_msg); *out_len = 0; return ""; } @@ -1387,29 +1410,32 @@ gdv_int32 ascii_utf8(const char* data, gdv_int32 data_len) { return static_cast(static_cast(data[0])); } -// Returns the ASCII character having the binary equivalent to A. -// If A is larger than 256 the result is equivalent to chr(A % 256). +// Returns the UTF-8 encoding of the Unicode code point A. +// Raises an error if A is not a valid code point, i.e. it is negative, greater +// than 0x10FFFF, or falls within the UTF-16 surrogate range 0xD800-0xDFFF. FORCE_INLINE -const char* chr_int32(gdv_int64 context, gdv_int32 in, gdv_int32* out_len) { - in = in % 256; - *out_len = 1; - - char* ret = reinterpret_cast(gdv_fn_context_arena_malloc(context, *out_len)); - if (ret == nullptr) { - gdv_fn_context_set_error_msg(context, "Could not allocate memory for output string"); +const char* chr_int64(gdv_int64 context, gdv_int64 in, gdv_int32* out_len) { + if (in < 0 || in > 0x10FFFF || (in >= 0xD800 && in <= 0xDFFF)) { + char err_msg[128]; + snprintf(err_msg, sizeof(err_msg), + "Input %" PRId64 + " is not a valid Unicode code point in the range 0 to 1114111, excluding " + "the surrogate range 0xD800–0xDFFF", + in); + gdv_fn_context_set_error_msg(context, err_msg); *out_len = 0; return ""; } - ret[0] = char(in); - return ret; -} -// Returns the ASCII character having the binary equivalent to A. -// If A is larger than 256 the result is equivalent to chr(A % 256). -FORCE_INLINE -const char* chr_int64(gdv_int64 context, gdv_int64 in, gdv_int32* out_len) { - in = in % 256; - *out_len = 1; + if (in <= 0x7F) { + *out_len = 1; + } else if (in <= 0x7FF) { + *out_len = 2; + } else if (in <= 0xFFFF) { + *out_len = 3; + } else { + *out_len = 4; + } char* ret = reinterpret_cast(gdv_fn_context_arena_malloc(context, *out_len)); if (ret == nullptr) { @@ -1417,10 +1443,36 @@ const char* chr_int64(gdv_int64 context, gdv_int64 in, gdv_int32* out_len) { *out_len = 0; return ""; } - ret[0] = char(in); + + switch (*out_len) { + case 1: + ret[0] = static_cast(in); + break; + case 2: + ret[0] = static_cast(0xC0 | (in >> 6)); + ret[1] = static_cast(0x80 | (in & 0x3F)); + break; + case 3: + ret[0] = static_cast(0xE0 | (in >> 12)); + ret[1] = static_cast(0x80 | ((in >> 6) & 0x3F)); + ret[2] = static_cast(0x80 | (in & 0x3F)); + break; + case 4: + ret[0] = static_cast(0xF0 | (in >> 18)); + ret[1] = static_cast(0x80 | ((in >> 12) & 0x3F)); + ret[2] = static_cast(0x80 | ((in >> 6) & 0x3F)); + ret[3] = static_cast(0x80 | (in & 0x3F)); + break; + } return ret; } +// Returns the UTF-8 encoding of the Unicode code point A. See chr_int64. +FORCE_INLINE +const char* chr_int32(gdv_int64 context, gdv_int32 in, gdv_int32* out_len) { + return chr_int64(context, in, out_len); +} + FORCE_INLINE const char* convert_fromUTF8_binary(gdv_int64 context, const char* bin_in, gdv_int32 len, gdv_int32* out_len) { @@ -1435,7 +1487,12 @@ const char* convert_replace_invalid_fromUTF8_binary(int64_t context, const char* int32_t char_to_replace_len, int32_t* out_len) { if (char_to_replace_len > 1) { - gdv_fn_context_set_error_msg(context, "Replacement of multiple bytes not supported"); + char err_msg[128]; + snprintf(err_msg, sizeof(err_msg), + "CONVERT_REPLACE_INVALID_FROM_UTF8: replacement must be a single byte, " + "got %d bytes", + char_to_replace_len); + gdv_fn_context_set_error_msg(context, err_msg); *out_len = 0; return ""; } @@ -1815,7 +1872,10 @@ gdv_int32 locate_utf8_utf8_int32(gdv_int64 context, const char* sub_str, gdv_int32 sub_str_len, const char* str, gdv_int32 str_len, gdv_int32 start_pos) { if (start_pos < 1) { - gdv_fn_context_set_error_msg(context, "Start position must be greater than 0"); + char err_msg[96]; + snprintf(err_msg, sizeof(err_msg), + "LOCATE: Start position must be greater than 0, got %d", start_pos); + gdv_fn_context_set_error_msg(context, err_msg); return 0; } @@ -1858,8 +1918,16 @@ const char* replace_with_max_len_utf8_utf8_utf8(gdv_int64 context, const char* t for (; text_index <= text_len - from_str_len;) { if (memcmp(text + text_index, from_str, from_str_len) == 0) { - if (out_index + text_index - last_match_index + to_str_len > max_length) { - gdv_fn_context_set_error_msg(context, "Buffer overflow for output string"); + // Compute the prospective length in gdv_int64: now that the wrapper may + // pass a max_length near INT_MAX, out_index can approach INT_MAX and a + // 32-bit sum would overflow before this guard runs -- precisely the case + // the guard exists to catch. (text_index - last_match_index) is a bounded + // non-negative span. + gdv_int64 prospective_len = static_cast(out_index) + + (text_index - last_match_index) + to_str_len; + if (prospective_len > max_length) { + gdv_fn_context_set_error_msg(context, + "REPLACE: Buffer overflow for output string"); *out_len = 0; return ""; } @@ -1893,8 +1961,9 @@ const char* replace_with_max_len_utf8_utf8_utf8(gdv_int64 context, const char* t return text; } - if (out_index + text_len - last_match_index > max_length) { - gdv_fn_context_set_error_msg(context, "Buffer overflow for output string"); + gdv_int64 final_len = static_cast(out_index) + (text_len - last_match_index); + if (final_len > max_length) { + gdv_fn_context_set_error_msg(context, "REPLACE: Buffer overflow for output string"); *out_len = 0; return ""; } @@ -1909,9 +1978,55 @@ const char* replace_utf8_utf8_utf8(gdv_int64 context, const char* text, gdv_int32 text_len, const char* from_str, gdv_int32 from_str_len, const char* to_str, gdv_int32 to_str_len, gdv_int32* out_len) { + // Size the output buffer so large results are not capped by an arbitrary + // limit, while avoiding a second pass over the input in the common case. + // - No replacement possible, or the result can only shrink/stay equal: + // text_len is a safe exact-or-upper bound, no scan. + // - Bounded-ratio expansion (per-match growth <= match length, upper bound + // fits within kMaxEagerAllocBytes): use an O(1) upper bound that assumes + // every position matches, skipping the match-counting scan. + // - Otherwise: count non-overlapping matches for the exact output size. + static constexpr gdv_int64 kMaxEagerAllocBytes = 32 * 1024 * 1024; // 32 MB + gdv_int64 max_length; + if (from_str_len <= 0 || from_str_len > text_len || to_str_len <= from_str_len) { + max_length = text_len; + } else { + gdv_int32 delta = to_str_len - from_str_len; // > 0 + gdv_int64 upper_bound = static_cast(text_len) + + (static_cast(text_len) / from_str_len) * delta; + if (delta <= from_str_len && upper_bound <= kMaxEagerAllocBytes) { + max_length = upper_bound; + } else { + gdv_int64 num_matches = 0; + for (gdv_int32 i = 0; i <= text_len - from_str_len;) { + if (memcmp(text + i, from_str, from_str_len) == 0) { + num_matches++; + i += from_str_len; + } else { + i++; + } + } + // No matches: the result is the input unchanged; return it without calling + // the helper (which would otherwise scan the text a second time). + if (num_matches == 0) { + *out_len = text_len; + return text; + } + max_length = static_cast(text_len) + num_matches * delta; + } + } + // Gandiva variable-length output uses int32 offsets, so a single output string + // cannot exceed INT_MAX bytes. Report this explicitly instead of letting the + // cast below wrap silently. + if (max_length > INT_MAX) { + gdv_fn_context_set_error_msg(context, + "REPLACE: output string exceeds maximum size of 2GB"); + *out_len = 0; + return ""; + } return replace_with_max_len_utf8_utf8_utf8(context, text, text_len, from_str, - from_str_len, to_str, to_str_len, 65535, - out_len); + from_str_len, to_str, to_str_len, + static_cast(max_length), out_len); } // Returns the quoted string (Includes escape character for any single quotes) @@ -2315,62 +2430,75 @@ const char* binary_string(gdv_int64 context, const char* text, gdv_int32 text_le return ret; } -#define CAST_INT_BIGINT_VARBINARY(OUT_TYPE, TYPE_NAME) \ - FORCE_INLINE \ - OUT_TYPE \ - cast##TYPE_NAME##_varbinary(gdv_int64 context, const char* in, int32_t in_len) { \ - if (in_len == 0) { \ - gdv_fn_context_set_error_msg(context, "Can't cast an empty string."); \ - return -1; \ - } \ - char sign = in[0]; \ - \ - bool negative = false; \ - if (sign == '-') { \ - negative = true; \ - /* Ignores the sign char in the hexadecimal string */ \ - in++; \ - in_len--; \ - } \ - \ - if (negative && in_len == 0) { \ - gdv_fn_context_set_error_msg(context, \ - "Can't cast hexadecimal with only a minus sign."); \ - return -1; \ - } \ - \ - OUT_TYPE result = 0; \ - int digit; \ - \ - int read_index = 0; \ - while (read_index < in_len) { \ - char c1 = in[read_index]; \ - if (isxdigit(c1)) { \ - digit = to_binary_from_hex(c1); \ - \ - OUT_TYPE next = result * 16 - digit; \ - \ - if (next > result) { \ - gdv_fn_context_set_error_msg(context, "Integer overflow."); \ - return -1; \ - } \ - result = next; \ - read_index++; \ - } else { \ - gdv_fn_context_set_error_msg(context, \ - "The hexadecimal given has invalid characters."); \ - return -1; \ - } \ - } \ - if (!negative) { \ - result *= -1; \ - \ - if (result < 0) { \ - gdv_fn_context_set_error_msg(context, "Integer overflow."); \ - return -1; \ - } \ - } \ - return result; \ +#define CAST_INT_BIGINT_VARBINARY(OUT_TYPE, TYPE_NAME) \ + FORCE_INLINE \ + OUT_TYPE \ + cast##TYPE_NAME##_varbinary(gdv_int64 context, const char* in, int32_t in_len) { \ + const char* in_original = in; \ + int32_t in_len_original = in_len; \ + if (in_len == 0) { \ + gdv_fn_context_set_error_msg( \ + context, "CAST_" #TYPE_NAME "_FROM_HEX: can't cast an empty string"); \ + return -1; \ + } \ + char sign = in[0]; \ + \ + bool negative = false; \ + if (sign == '-') { \ + negative = true; \ + /* Ignores the sign char in the hexadecimal string */ \ + in++; \ + in_len--; \ + } \ + \ + if (negative && in_len == 0) { \ + gdv_fn_context_set_error_msg( \ + context, "CAST_" #TYPE_NAME \ + "_FROM_HEX: can't cast hexadecimal with only a minus sign"); \ + return -1; \ + } \ + \ + OUT_TYPE result = 0; \ + int digit; \ + \ + int read_index = 0; \ + while (read_index < in_len) { \ + char c1 = in[read_index]; \ + if (isxdigit(c1)) { \ + digit = to_binary_from_hex(c1); \ + \ + OUT_TYPE next = result * 16 - digit; \ + \ + if (next > result) { \ + std::string err_msg = \ + "CAST_" #TYPE_NAME \ + "_FROM_HEX: integer overflow while reading hex value '" + \ + std::string(in_original, in_len_original) + "'"; \ + gdv_fn_context_set_error_msg(context, err_msg.c_str()); \ + return -1; \ + } \ + result = next; \ + read_index++; \ + } else { \ + std::string err_msg = "CAST_" #TYPE_NAME \ + "_FROM_HEX: invalid character in hex value '" + \ + std::string(in_original, in_len_original) + "'"; \ + gdv_fn_context_set_error_msg(context, err_msg.c_str()); \ + return -1; \ + } \ + } \ + if (!negative) { \ + result *= -1; \ + \ + if (result < 0) { \ + std::string err_msg = "CAST_" #TYPE_NAME \ + "_FROM_HEX: integer overflow while reading hex value '" + \ + std::string(in_original, in_len_original) + "'"; \ + gdv_fn_context_set_error_msg(context, err_msg.c_str()); \ + return -1; \ + } \ + } \ + return result; \ } CAST_INT_BIGINT_VARBINARY(int32_t, INT) diff --git a/cpp/src/gandiva/precompiled/string_ops_test.cc b/cpp/src/gandiva/precompiled/string_ops_test.cc index ea661585ecb5..ff7961ffc86b 100644 --- a/cpp/src/gandiva/precompiled/string_ops_test.cc +++ b/cpp/src/gandiva/precompiled/string_ops_test.cc @@ -56,11 +56,12 @@ TEST(TestStringOps, TestAscii) { } TEST(TestStringOps, TestChrBigInt) { - // CHR + // CHR returns the UTF-8 encoding of the given Unicode code point. gandiva::ExecutionContext ctx; uint64_t ctx_ptr = reinterpret_cast(&ctx); int32_t out_len = 0; + // 1-byte ASCII code points. auto out = chr_int32(ctx_ptr, 88, &out_len); EXPECT_EQ(std::string(out, out_len), "X"); @@ -70,62 +71,89 @@ TEST(TestStringOps, TestChrBigInt) { out = chr_int32(ctx_ptr, 49, &out_len); EXPECT_EQ(std::string(out, out_len), "1"); - out = chr_int64(ctx_ptr, 84, &out_len); - EXPECT_EQ(std::string(out, out_len), "T"); + out = chr_int32(ctx_ptr, 33, &out_len); + EXPECT_EQ(std::string(out, out_len), "!"); - out = chr_int32(ctx_ptr, 340, &out_len); - EXPECT_EQ(std::string(out, out_len), "T"); + out = chr_int64(ctx_ptr, 0, &out_len); + EXPECT_EQ(std::string(out, out_len), std::string("\0", 1)); - out = chr_int64(ctx_ptr, 256, &out_len); - EXPECT_EQ(std::strcmp(out, "\0"), 0); + // BACKSPACE + out = chr_int64(ctx_ptr, 8, &out_len); + EXPECT_EQ(std::string(out, out_len), "\b"); - out = chr_int32(ctx_ptr, 33, &out_len); - EXPECT_EQ(std::string(out, out_len), "!"); + // ESCAPE (ESC) + out = chr_int64(ctx_ptr, 27, &out_len); + EXPECT_EQ(std::string(out, out_len), "\x1B"); - out = chr_int64(ctx_ptr, 46, &out_len); - EXPECT_EQ(std::string(out, out_len), "."); + // Highest 1-byte code point (U+007F, DELETE). + out = chr_int32(ctx_ptr, 0x7F, &out_len); + EXPECT_EQ(std::string(out, out_len), "\x7F"); - out = chr_int32(ctx_ptr, 63, &out_len); - EXPECT_EQ(std::string(out, out_len), "?"); + // 2-byte code points. + // Lowest 2-byte code point (U+0080). + out = chr_int32(ctx_ptr, 0x80, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xC2\x80"); - out = chr_int64(ctx_ptr, 0, &out_len); - EXPECT_EQ(std::strcmp(out, "\0"), 0); + // í (U+00ED) + out = chr_int32(ctx_ptr, 237, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xC3\xAD"); - out = chr_int32(ctx_ptr, -158, &out_len); - EXPECT_EQ(std::string(out, out_len), "b"); + // Highest 2-byte code point (U+07FF). + out = chr_int64(ctx_ptr, 0x7FF, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xDF\xBF"); - out = chr_int64(ctx_ptr, -5, &out_len); - EXPECT_EQ(std::string(out, out_len), "\xFB"); + // 3-byte code points. + // Lowest 3-byte code point (U+0800). + out = chr_int32(ctx_ptr, 0x800, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xE0\xA0\x80"); - out = chr_int32(ctx_ptr, -340, &out_len); - EXPECT_EQ(std::string(out, out_len), "\xAC"); + // € (U+20AC) + out = chr_int32(ctx_ptr, 8364, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xE2\x82\xAC"); - out = chr_int64(ctx_ptr, -66, &out_len); - EXPECT_EQ(std::string(out, out_len), "\xBE"); + // 日 (U+65E5) + out = chr_int64(ctx_ptr, 26085, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xE6\x97\xA5"); - //€ - out = chr_int32(ctx_ptr, 128, &out_len); - EXPECT_EQ(std::string(out, out_len), "\x80"); + // Highest 3-byte code point (U+FFFF). + out = chr_int32(ctx_ptr, 0xFFFF, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xEF\xBF\xBF"); - //œ - out = chr_int64(ctx_ptr, 156, &out_len); - EXPECT_EQ(std::string(out, out_len), "\x9C"); + // 4-byte code points. + // Lowest 4-byte code point (U+10000). + out = chr_int64(ctx_ptr, 0x10000, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xF0\x90\x80\x80"); - //ÿ - out = chr_int32(ctx_ptr, 255, &out_len); - EXPECT_EQ(std::string(out, out_len), "\xFF"); + // 😀 (U+1F600) + out = chr_int64(ctx_ptr, 0x1F600, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xF0\x9F\x98\x80"); - // BACKSPACE - out = chr_int64(ctx_ptr, 8, &out_len); - EXPECT_EQ(std::string(out, out_len), "\b"); + // Highest valid code point (U+10FFFF). + out = chr_int64(ctx_ptr, 0x10FFFF, &out_len); + EXPECT_EQ(std::string(out, out_len), "\xF4\x8F\xBF\xBF"); - // DEVICE CONTROL 3 (DC3) - out = chr_int32(ctx_ptr, 19, &out_len); - EXPECT_EQ(std::string(out, out_len), "\x13"); + EXPECT_FALSE(ctx.has_error()); - // ESCAPE (ESC) - out = chr_int64(ctx_ptr, 27, &out_len); - EXPECT_EQ(std::string(out, out_len), "\x1B"); + // Invalid code points raise an error that includes the offending value. + chr_int64(ctx_ptr, -1, &out_len); + EXPECT_EQ(out_len, 0); + EXPECT_TRUE(ctx.get_error().find("not a valid Unicode code point") != std::string::npos) + << ctx.get_error(); + EXPECT_TRUE(ctx.get_error().find("-1") != std::string::npos) << ctx.get_error(); + ctx.Reset(); + + chr_int64(ctx_ptr, 0x110000, &out_len); + EXPECT_EQ(out_len, 0); + EXPECT_TRUE(ctx.get_error().find("not a valid Unicode code point") != std::string::npos) + << ctx.get_error(); + ctx.Reset(); + + // UTF-16 surrogate range is not a valid code point. + chr_int32(ctx_ptr, 0xD800, &out_len); + EXPECT_EQ(out_len, 0); + EXPECT_TRUE(ctx.get_error().find("not a valid Unicode code point") != std::string::npos) + << ctx.get_error(); + ctx.Reset(); } TEST(TestStringOps, TestBeginsEnds) { @@ -1971,6 +1999,68 @@ TEST(TestStringOps, TestReplace) { EXPECT_EQ(std::string(out_str, out_len), "TestString"); EXPECT_FALSE(ctx.has_error()); + // No match on the large-expansion (counting) path: from "z" to "zzz" expands + // by more than from_len, so this exercises the count branch's zero-match + // early return. + out_str = replace_utf8_utf8_utf8(ctx_ptr, "TestString", 10, "z", 1, "zzz", 3, &out_len); + EXPECT_EQ(std::string(out_str, out_len), "TestString"); + EXPECT_FALSE(ctx.has_error()); + + // Large output (>64 KB) must not overflow: buffer is sized to the exact result. + std::string large_in(35000, 'X'); + std::string large_expected(70000, '\0'); + for (int i = 0; i < 35000; ++i) { + large_expected[2 * i] = 'X'; + large_expected[2 * i + 1] = 'Y'; + } + out_str = replace_utf8_utf8_utf8(ctx_ptr, large_in.data(), + static_cast(large_in.size()), "X", 1, "XY", 2, + &out_len); + EXPECT_EQ(out_len, 70000); + EXPECT_EQ(std::string(out_str, out_len), large_expected); + EXPECT_FALSE(ctx.has_error()); + + // Large shrinking output ("XX" -> "X") on a >64 KB input. + std::string large_shrink_in(70000, 'X'); + std::string large_shrink_expected(35000, 'X'); + out_str = replace_utf8_utf8_utf8(ctx_ptr, large_shrink_in.data(), + static_cast(large_shrink_in.size()), "XX", 2, + "X", 1, &out_len); + EXPECT_EQ(out_len, 35000); + EXPECT_EQ(std::string(out_str, out_len), large_shrink_expected); + EXPECT_FALSE(ctx.has_error()); + + // Edge case: result size of exactly 0 (every byte of text is removed). Takes + // the no-scan shrink path (to_str_len <= from_str_len). + out_str = replace_utf8_utf8_utf8(ctx_ptr, "aaaa", 4, "a", 1, "", 0, &out_len); + EXPECT_EQ(out_len, 0); + EXPECT_EQ(std::string(out_str, out_len), ""); + EXPECT_FALSE(ctx.has_error()); + + // Edge case: result size one past the INT_MAX boundary. 65536 single-char + // matches each expanding to 32768 bytes gives max_length = 65536 * 32768 = + // 2^31 = INT_MAX + 1, so it is reported cleanly (guard fires before any alloc). + std::string boundary_in(65536, 'a'); + std::string boundary_to(32768, 'b'); + replace_utf8_utf8_utf8( + ctx_ptr, boundary_in.data(), static_cast(boundary_in.size()), "a", 1, + boundary_to.data(), static_cast(boundary_to.size()), &out_len); + EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("exceeds maximum size")); + EXPECT_EQ(out_len, 0); + ctx.Reset(); + + // Output that would exceed INT_MAX (2GB) is reported cleanly rather than + // silently wrapping the int32 size. 50000 matches each expanding to 50000 + // bytes implies max_length = 2.5e9; the guard fires before any large alloc. + std::string huge_in(50000, 'X'); + std::string huge_to(50000, 'Z'); + replace_utf8_utf8_utf8(ctx_ptr, huge_in.data(), static_cast(huge_in.size()), + "X", 1, huge_to.data(), static_cast(huge_to.size()), + &out_len); + EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("exceeds maximum size")); + EXPECT_EQ(out_len, 0); + ctx.Reset(); + replace_with_max_len_utf8_utf8_utf8(ctx_ptr, "Hell", 4, "ell", 3, "ollow", 5, 5, &out_len); EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("Buffer overflow for output string")); diff --git a/cpp/src/gandiva/precompiled/time.cc b/cpp/src/gandiva/precompiled/time.cc index 5ab2e20faa9a..50833c89fec3 100644 --- a/cpp/src/gandiva/precompiled/time.cc +++ b/cpp/src/gandiva/precompiled/time.cc @@ -17,6 +17,8 @@ #include "./epoch_time_point.h" +#include + extern "C" { #define __STDC_FORMAT_MACROS @@ -268,7 +270,9 @@ static const int WEEK_LEN[] = {6, 6, 7, 9, 8, 6, 8}; } \ } \ if (dateSearch == 0) { \ - gdv_fn_context_set_error_msg(context, "The weekday in this entry is invalid"); \ + std::string err_msg = "NEXT_DAY: '" + std::string(in, in_len) + \ + "' is not a recognized day of the week"; \ + gdv_fn_context_set_error_msg(context, err_msg.c_str()); \ return 0; \ } \ \ @@ -1052,7 +1056,12 @@ CAST_NULLABLE_INTERVAL_DAY(int64) gdv_month_interval castNULLABLEINTERVALYEAR_##TYPE(int64_t context, gdv_##TYPE in) { \ gdv_month_interval value = static_cast(in); \ if (value != in) { \ - gdv_fn_context_set_error_msg(context, "Integer overflow"); \ + char err_msg[96]; \ + snprintf(err_msg, sizeof(err_msg), \ + "CAST_INTERVAL_YEAR: Integer overflow casting %" PRId64 \ + " to month interval", \ + static_cast(in)); \ + gdv_fn_context_set_error_msg(context, err_msg); \ } \ return value; \ } diff --git a/cpp/src/gandiva/precompiled/time_test.cc b/cpp/src/gandiva/precompiled/time_test.cc index 6cfa6acf579d..4310490a228b 100644 --- a/cpp/src/gandiva/precompiled/time_test.cc +++ b/cpp/src/gandiva/precompiled/time_test.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include "arrow/util/logging_internal.h" @@ -962,10 +963,80 @@ TEST(TestTime, TestNextDay) { ts = StringToTimestamp("2015-08-06 11:12:30"); out = next_day_from_timestamp(context_ptr, ts, "AHSRK", 5); - EXPECT_EQ(context.get_error(), "The weekday in this entry is invalid"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("NEXT_DAY")); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("AHSRK")); context.Reset(); } +// Document that next_day's weekday-name matching is case-sensitive: +// the WEEK[] lookup table holds uppercase names ("MONDAY", "TUE", ...) and +// is_substr_utf8_utf8 does a byte-exact memcmp, so lowercase or mixed-case +// input does not match and produces a NEXT_DAY error. +TEST(TestTime, TestNextDayCaseSensitive) { + ExecutionContext context; + int64_t context_ptr = reinterpret_cast(&context); + + gdv_timestamp ts = StringToTimestamp("2021-11-08 10:20:34"); + + // Uppercase: matches. + auto out = next_day_from_timestamp(context_ptr, ts, "FRIDAY", 6); + EXPECT_EQ(StringToTimestamp("2021-11-12 00:00:00"), out); + EXPECT_FALSE(context.has_error()); + + // Lowercase: does NOT match (case-sensitive memcmp against uppercase WEEK[]). + out = next_day_from_timestamp(context_ptr, ts, "friday", 6); + EXPECT_TRUE(context.has_error()); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("NEXT_DAY")); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("friday")); + context.Reset(); + + // Mixed case: also does NOT match. + out = next_day_from_timestamp(context_ptr, ts, "Friday", 6); + EXPECT_TRUE(context.has_error()); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("NEXT_DAY")); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("Friday")); + context.Reset(); +} + +// Document that next_day's weekday-name matching is loose: it uses +// is_substr_utf8_utf8 to test whether the input is a *substring* of any +// WEEK[] entry, walking the array in the order +// SUNDAY, MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY +// and returning the FIRST match. This means single-letter prefixes 'S' and +// 'T' are ambiguous and silently resolve to SUNDAY / TUESDAY respectively +// (never SATURDAY / THURSDAY). Likewise, any substring shared across weekdays +// (e.g. "DAY") matches SUNDAY because it is first in the array. +TEST(TestTime, TestNextDayAmbiguousPrefix) { + ExecutionContext context; + int64_t context_ptr = reinterpret_cast(&context); + + gdv_timestamp ts = StringToTimestamp("2021-11-08 10:20:34"); // Mon + + // "S" -> Sunday (could have been Saturday). + auto out = next_day_from_timestamp(context_ptr, ts, "S", 1); + EXPECT_EQ(StringToTimestamp("2021-11-14 00:00:00"), out); // next Sunday + EXPECT_FALSE(context.has_error()); + + // "T" -> Tuesday (could have been Thursday). + out = next_day_from_timestamp(context_ptr, ts, "T", 1); + EXPECT_EQ(StringToTimestamp("2021-11-09 00:00:00"), out); // next Tuesday + EXPECT_FALSE(context.has_error()); + + // "DAY" appears in every weekday name -> matches SUNDAY (first in array). + out = next_day_from_timestamp(context_ptr, ts, "DAY", 3); + EXPECT_EQ(StringToTimestamp("2021-11-14 00:00:00"), out); // next Sunday + EXPECT_FALSE(context.has_error()); + + // Unambiguous 2-letter prefixes work as expected. + out = next_day_from_timestamp(context_ptr, ts, "SA", 2); + EXPECT_EQ(StringToTimestamp("2021-11-13 00:00:00"), out); // next Saturday + EXPECT_FALSE(context.has_error()); + + out = next_day_from_timestamp(context_ptr, ts, "TH", 2); + EXPECT_EQ(StringToTimestamp("2021-11-11 00:00:00"), out); // next Thursday + EXPECT_FALSE(context.has_error()); +} + TEST(TestTime, TestCastTimestampToTime) { gdv_timestamp ts = StringToTimestamp("2000-05-01 10:20:34"); auto expected_response = @@ -1172,7 +1243,7 @@ TEST(TestTime, TestCastNullableInterval) { EXPECT_EQ(castNULLABLEINTERVALYEAR_int64(context_ptr, 1201), 1201); // validate overflow error when using bigint as input castNULLABLEINTERVALYEAR_int64(context_ptr, INT64_MAX); - EXPECT_EQ(context.get_error(), "Integer overflow"); + EXPECT_THAT(context.get_error(), ::testing::HasSubstr("Integer overflow")); context.Reset(); } diff --git a/cpp/src/gandiva/regex_functions_holder.cc b/cpp/src/gandiva/regex_functions_holder.cc index 1c9e44d61b23..177bce9bf696 100644 --- a/cpp/src/gandiva/regex_functions_holder.cc +++ b/cpp/src/gandiva/regex_functions_holder.cc @@ -213,8 +213,8 @@ void ReplaceHolder::return_error(ExecutionContext* context, std::string& data, } Result> ExtractHolder::Make(const FunctionNode& node) { - ARROW_RETURN_IF(node.children().size() != 3, - Status::Invalid("'extract' function requires three parameters")); + ARROW_RETURN_IF(node.children().size() != 2 && node.children().size() != 3, + Status::Invalid("'extract' function requires two or three parameters")); auto literal = dynamic_cast(node.children().at(1).get()); ARROW_RETURN_IF( @@ -238,7 +238,11 @@ const char* ExtractHolder::operator()(ExecutionContext* ctx, const char* user_in int32_t user_input_len, int32_t extract_index, int32_t* out_length) { if (extract_index < 0 || extract_index >= num_groups_pattern_) { - ctx->set_error_msg("Index to extract out of range"); + std::string err_msg = "REGEXP_EXTRACT: invalid group_index '" + + std::to_string(extract_index) + "'; must be between 0 and " + + std::to_string(num_groups_pattern_ - 1) + + " (the number of capture groups in the pattern)"; + ctx->set_error_msg(err_msg.c_str()); *out_length = 0; return ""; } diff --git a/cpp/src/gandiva/regex_functions_holder_test.cc b/cpp/src/gandiva/regex_functions_holder_test.cc index d0cef5068749..14992d10d04e 100644 --- a/cpp/src/gandiva/regex_functions_holder_test.cc +++ b/cpp/src/gandiva/regex_functions_holder_test.cc @@ -605,24 +605,100 @@ TEST_F(TestExtractHolder, TestExtractInvalidPattern) { execution_context_.Reset(); } -TEST_F(TestExtractHolder, TestErrorWhileBuildingHolder) { - // Create function with incorrect number of params +TEST_F(TestExtractHolder, TestEmptyInput) { + EXPECT_OK_AND_ASSIGN(auto extract_holder, ExtractHolder::Make(R"((\w+))")); + auto& extract = *extract_holder; + int32_t out_length = 0; + + const char* ret = extract(&execution_context_, "", 0, 0, &out_length); + EXPECT_EQ(std::string(ret, out_length), ""); + EXPECT_FALSE(execution_context_.has_error()); +} + +TEST_F(TestExtractHolder, TestOptionalGroup) { + // (a)?(b): group 1 is optional; when input is "b" it doesn't participate + EXPECT_OK_AND_ASSIGN(auto extract_holder, ExtractHolder::Make(R"((a)?(b))")); + auto& extract = *extract_holder; + int32_t out_length = 0; + + std::string input = "b"; + const char* ret = extract(&execution_context_, input.c_str(), + static_cast(input.size()), 1, &out_length); + EXPECT_EQ(std::string(ret, out_length), ""); + EXPECT_FALSE(execution_context_.has_error()); + + ret = extract(&execution_context_, input.c_str(), static_cast(input.size()), 2, + &out_length); + EXPECT_EQ(std::string(ret, out_length), "b"); + + input = "ab"; + ret = extract(&execution_context_, input.c_str(), static_cast(input.size()), 1, + &out_length); + EXPECT_EQ(std::string(ret, out_length), "a"); +} + +TEST_F(TestExtractHolder, TestNoUserGroups) { + // Pattern with no user capturing groups — only the outer wrapper group exists. + // Index 0 returns the full match; index 1 is out of range. + EXPECT_OK_AND_ASSIGN(auto extract_holder, ExtractHolder::Make(R"(\d+)")); + auto& extract = *extract_holder; + int32_t out_length = 0; + + std::string input = "abc123def"; + const char* ret = extract(&execution_context_, input.c_str(), + static_cast(input.size()), 0, &out_length); + EXPECT_EQ(std::string(ret, out_length), "123"); + EXPECT_FALSE(execution_context_.has_error()); + + ret = extract(&execution_context_, input.c_str(), static_cast(input.size()), 1, + &out_length); + EXPECT_EQ(out_length, 0); + EXPECT_TRUE(execution_context_.has_error()); + execution_context_.Reset(); +} + +TEST_F(TestExtractHolder, TestDefaultIndexExtract) { + // 2-arg form defaults to index 1 (first capture group) auto field = std::make_shared(arrow::field("in", arrow::utf8())); auto pattern_node = std::make_shared( arrow::utf8(), LiteralHolder(R"((\w+) (\w+))"), false); auto function_node = FunctionNode("regexp_extract", {field, pattern_node}, arrow::utf8()); + EXPECT_OK_AND_ASSIGN(auto extract_holder, ExtractHolder::Make(function_node)); + + std::string input_string = "John Doe"; + int32_t out_length = 0; + + auto& extract = *extract_holder; + const char* ret = extract(&execution_context_, input_string.c_str(), + static_cast(input_string.length()), 1, &out_length); + EXPECT_EQ(std::string(ret, out_length), "John"); + + input_string = "Ringo Beast"; + ret = extract(&execution_context_, input_string.c_str(), + static_cast(input_string.length()), 1, &out_length); + EXPECT_EQ(std::string(ret, out_length), "Ringo"); +} + +TEST_F(TestExtractHolder, TestErrorWhileBuildingHolder) { + // Create function with incorrect number of params (one arg) + auto field = std::make_shared(arrow::field("in", arrow::utf8())); + NodeVector one_arg = {field}; + auto function_node = FunctionNode("regexp_extract", one_arg, arrow::utf8()); + auto extract_holder = ExtractHolder::Make(function_node); EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, ::testing::HasSubstr("'extract' function requires three parameters"), + Invalid, + ::testing::HasSubstr("'extract' function requires two or three parameters"), extract_holder.status()); execution_context_.Reset(); // Create function with non-utf8 literal parameter as pattern field = std::make_shared(arrow::field("in", arrow::utf8())); - pattern_node = std::make_shared(arrow::int32(), LiteralHolder(2), false); + auto pattern_node = + std::make_shared(arrow::int32(), LiteralHolder(2), false); auto index_node = std::make_shared(arrow::field("idx", arrow::int32())); function_node = FunctionNode("regexp_extract", {field, pattern_node, index_node}, arrow::utf8()); @@ -655,3 +731,60 @@ TEST_F(TestExtractHolder, TestErrorWhileBuildingHolder) { } } // namespace gandiva + +extern "C" const char* gdv_fn_regexp_extract_utf8_utf8(int64_t ptr, int64_t holder_ptr, + const char* data, int32_t data_len, + const char* pattern, + int32_t pattern_len, + int32_t* out_length); + +TEST(TestRegexpExtractStub, TestDefaultIndexStub) { + gandiva::ExecutionContext ctx; + auto ctx_ptr = reinterpret_cast(&ctx); + + EXPECT_OK_AND_ASSIGN(auto holder, gandiva::ExtractHolder::Make(R"((\w+) (\w+))")); + auto holder_ptr = reinterpret_cast(holder.get()); + + std::string pattern = R"((\w+) (\w+))"; + int32_t out_length = 0; + + std::string input = "John Doe"; + const char* ret = gdv_fn_regexp_extract_utf8_utf8( + ctx_ptr, holder_ptr, input.c_str(), static_cast(input.size()), + pattern.c_str(), static_cast(pattern.size()), &out_length); + EXPECT_EQ(std::string(ret, out_length), "John"); + + input = "Ringo Beast"; + ret = gdv_fn_regexp_extract_utf8_utf8( + ctx_ptr, holder_ptr, input.c_str(), static_cast(input.size()), + pattern.c_str(), static_cast(pattern.size()), &out_length); + EXPECT_EQ(std::string(ret, out_length), "Ringo"); + + // no match returns empty string + input = "--- ---"; + ret = gdv_fn_regexp_extract_utf8_utf8( + ctx_ptr, holder_ptr, input.c_str(), static_cast(input.size()), + pattern.c_str(), static_cast(pattern.size()), &out_length); + EXPECT_EQ(out_length, 0); +} + +extern "C" const char* gdv_fn_regexp_extract_utf8_utf8_int32( + int64_t ptr, int64_t holder_ptr, const char* data, int32_t data_len, + const char* pattern, int32_t pattern_len, int32_t extract_index, int32_t* out_length); + +TEST(TestRegexpExtractStub, TestIndexStub) { + gandiva::ExecutionContext ctx; + auto ctx_ptr = reinterpret_cast(&ctx); + + EXPECT_OK_AND_ASSIGN(auto holder, gandiva::ExtractHolder::Make(R"((\w+) (\w+))")); + auto holder_ptr = reinterpret_cast(holder.get()); + + std::string pattern = R"((\w+) (\w+))"; + int32_t out_length = 0; + + std::string input = "John Doe"; + const char* ret = gdv_fn_regexp_extract_utf8_utf8_int32( + ctx_ptr, holder_ptr, input.c_str(), static_cast(input.size()), + pattern.c_str(), static_cast(pattern.size()), 2, &out_length); + EXPECT_EQ(std::string(ret, out_length), "Doe"); +} diff --git a/cpp/src/gandiva/tests/CMakeLists.txt b/cpp/src/gandiva/tests/CMakeLists.txt index 356b976e0058..be635e16d3ad 100644 --- a/cpp/src/gandiva/tests/CMakeLists.txt +++ b/cpp/src/gandiva/tests/CMakeLists.txt @@ -51,6 +51,24 @@ if(ARROW_BUILD_STATIC) "gandiva" EXTRA_LINK_LIBS gandiva_static) + + # Calls the precompiled REPLACE functions directly, so it compiles + # string_ops.cc/context_helper.cc with GANDIVA_UNIT_TEST=1 (which exposes them + # as linkable symbols). Only built when ARROW_BUILD_BENCHMARKS is ON. + add_arrow_benchmark(string_ops_benchmark + SOURCES + string_ops_benchmark.cc + ../precompiled/string_ops.cc + PREFIX + "gandiva" + EXTRA_LINK_LIBS + gandiva_static) + if(TARGET gandiva-string-ops-benchmark) + target_compile_definitions(gandiva-string-ops-benchmark + PRIVATE GANDIVA_UNIT_TEST=1 ARROW_STATIC GANDIVA_STATIC) + target_include_directories(gandiva-string-ops-benchmark SYSTEM + PRIVATE ${CMAKE_SOURCE_DIR}/src) + endif() endif() add_subdirectory(external_functions) diff --git a/cpp/src/gandiva/tests/decimal_test.cc b/cpp/src/gandiva/tests/decimal_test.cc index f8d049fd805b..043bdc4605a7 100644 --- a/cpp/src/gandiva/tests/decimal_test.cc +++ b/cpp/src/gandiva/tests/decimal_test.cc @@ -976,6 +976,41 @@ TEST_F(TestDecimal, TestCastVarCharDecimal) { EXPECT_ARROW_ARRAY_EQUALS(exp, outputs[1]); } +// Regression test for GH-50140: castVARCHAR(decimal) must fail gracefully instead +// of corrupting native memory (SIGSEGV) when given an invalid output length. +TEST_F(TestDecimal, TestCastVarCharDecimalNegativeLength) { + constexpr int32_t precision = 38; + constexpr int32_t scale = 2; + auto decimal_type = std::make_shared(precision, scale); + + auto field_dec = field("dec", decimal_type); + auto schema = arrow::schema({field_dec}); + auto field_res_str = field("res_str", utf8()); + + auto node_dec = TreeExprBuilder::MakeField(field_dec); + // A negative output length must not be used as a memcpy size. + auto neg_len = TreeExprBuilder::MakeLiteral(static_cast(-1)); + auto cast_varchar = + TreeExprBuilder::MakeFunction("castVARCHAR", {node_dec, neg_len}, utf8()); + auto expr = TreeExprBuilder::MakeExpression(cast_varchar, field_res_str); + + std::shared_ptr projector; + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); + + auto array_dec = + MakeArrowArrayDecimal(decimal_type, MakeDecimalVector({"10.51"}, scale), {true}); + auto in_batch = arrow::RecordBatch::Make(schema, 1, {array_dec}); + + arrow::ArrayVector outputs; + status = projector->Evaluate(*in_batch, pool_, &outputs); + // The evaluation should report a graceful error rather than crash. + EXPECT_FALSE(status.ok()) << status.message(); + EXPECT_NE(status.message().find("Output buffer length can't be negative"), + std::string::npos) + << status.message(); +} + TEST_F(TestDecimal, TestCastDecimalVarChar) { // schema for input fields constexpr int32_t precision = 4; diff --git a/cpp/src/gandiva/tests/projector_test.cc b/cpp/src/gandiva/tests/projector_test.cc index 268cb55a6422..e613df8ad7c7 100644 --- a/cpp/src/gandiva/tests/projector_test.cc +++ b/cpp/src/gandiva/tests/projector_test.cc @@ -1317,13 +1317,16 @@ TEST_F(TestProjector, TestChr) { auto status = Projector::Make(schema, {chr_expr}, TestConfiguration(), &projector); EXPECT_TRUE(status.ok()) << status.message(); - // Create a row-batch with some sample data + // Create a row-batch with code points spanning 1- to 4-byte UTF-8 encodings. + // 65 -> "A", 237 -> "í" (U+00ED), 8364 -> "€" (U+20AC), 26085 -> "日" (U+65E5), + // 128512 -> "😀" (U+1F600). int num_records = 5; auto array0 = - MakeArrowArrayInt64({65, 84, 255, 340, -5}, {true, true, true, true, true}); - // expected output - auto exp_chr = - MakeArrowArrayUtf8({"A", "T", "\xFF", "T", "\xFB"}, {true, true, true, true, true}); + MakeArrowArrayInt64({65, 237, 8364, 26085, 128512}, {true, true, true, true, true}); + // expected UTF-8 output + auto exp_chr = MakeArrowArrayUtf8( + {"A", "\xC3\xAD", "\xE2\x82\xAC", "\xE6\x97\xA5", "\xF0\x9F\x98\x80"}, + {true, true, true, true, true}); // prepare input record batch auto in_batch = arrow::RecordBatch::Make(schema, num_records, {array0}); @@ -1337,6 +1340,37 @@ TEST_F(TestProjector, TestChr) { EXPECT_ARROW_ARRAY_EQUALS(exp_chr, outputs.at(0)); } +TEST_F(TestProjector, TestChrInvalidCodePoint) { + // schema for input fields + auto field0 = field("f0", int64()); + auto schema = arrow::schema({field0}); + + // output fields + auto field_chr = field("chr", arrow::utf8()); + + // Build expression + auto chr_expr = TreeExprBuilder::MakeExpression("chr", {field0}, field_chr); + + std::shared_ptr projector; + auto status = Projector::Make(schema, {chr_expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); + + // A code point outside the valid Unicode range (here, a negative value) must + // fail evaluation rather than wrap around. + int num_records = 1; + auto array0 = MakeArrowArrayInt64({-5}, {true}); + auto in_batch = arrow::RecordBatch::Make(schema, num_records, {array0}); + + // Evaluate expression + arrow::ArrayVector outputs; + status = projector->Evaluate(*in_batch, pool_, &outputs); + EXPECT_FALSE(status.ok()); + EXPECT_NE(status.message().find("not a valid Unicode code point"), std::string::npos) + << status.message(); + // The message should include the offending value for debuggability. + EXPECT_NE(status.message().find("-5"), std::string::npos) << status.message(); +} + TEST_F(TestProjector, TestBase64) { // schema for input fields auto field0 = field("f0", arrow::binary()); @@ -3024,6 +3058,47 @@ TEST_F(TestProjector, TestRegexpExtract) { EXPECT_ARROW_ARRAY_EQUALS(exp_extract, outputs.at(0)); } +TEST_F(TestProjector, TestRegexpExtractTwoArg) { + // schema for input fields + auto field0 = field("f0", arrow::utf8()); + auto schema = arrow::schema({field0}); + + // output fields + auto field_extract = field("extract", arrow::utf8()); + + // The two-arg overload defaults to extracting the first capture group (index 1). + std::string pattern(R"((\w+) (\w+))"); + auto literal = TreeExprBuilder::MakeStringLiteral(pattern); + auto node0 = TreeExprBuilder::MakeField(field0); + + // Build expression with the two-arg overload: regexp_extract(string, pattern) + auto regexp_extract_func = + TreeExprBuilder::MakeFunction("regexp_extract", {node0, literal}, arrow::utf8()); + auto extract_expr = TreeExprBuilder::MakeExpression(regexp_extract_func, field_extract); + + std::shared_ptr projector; + auto status = Projector::Make(schema, {extract_expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); + + // Create a row-batch with some sample data + int num_records = 3; + auto array0 = MakeArrowArrayUtf8({"John Doe", "Ringo Beast", "stringthatdonotmatch"}, + {true, true, true}); + // expected output: first capture group, empty string when the pattern does not match + auto exp_extract = MakeArrowArrayUtf8({"John", "Ringo", ""}, {true, true, true}); + + // prepare input record batch + auto in = arrow::RecordBatch::Make(schema, num_records, {array0}); + + // Evaluate expression + arrow::ArrayVector outputs; + status = projector->Evaluate(*in, pool_, &outputs); + EXPECT_TRUE(status.ok()) << status.message(); + + // Validate results + EXPECT_ARROW_ARRAY_EQUALS(exp_extract, outputs.at(0)); +} + TEST_F(TestProjector, TestCastVarbinary) { auto field0 = field("f0", arrow::utf8()); auto field1 = field("f1", arrow::int64()); diff --git a/cpp/src/gandiva/tests/string_ops_benchmark.cc b/cpp/src/gandiva/tests/string_ops_benchmark.cc new file mode 100644 index 000000000000..4b8b82d08a41 --- /dev/null +++ b/cpp/src/gandiva/tests/string_ops_benchmark.cc @@ -0,0 +1,162 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Microbenchmark comparing the current REPLACE implementation against the +// pre-change one, to measure the cost of the match-counting scan the fix added +// to size the output buffer. +// +// BM_ReplaceNew = replace_utf8_utf8_utf8 (upper bound or counting scan, then a +// single write pass) +// BM_ReplaceOld = replace_with_max_len_utf8_utf8_utf8(..., capacity, ...) given +// an exact buffer: the pre-change algorithm with no counting +// scan. Compare the two rows per case to read the scan's cost. +// +// Unlike the projector-level micro_benchmarks, this calls the precompiled +// functions directly, so the build compiles string_ops.cc with GANDIVA_UNIT_TEST. + +#include +#include +#include +#include + +#include "benchmark/benchmark.h" + +#include "gandiva/execution_context.h" +#include "gandiva/precompiled/types.h" + +namespace gandiva { +namespace { + +struct ReplaceCase { + const char* name; + int64_t text_len; + int stride; // a match (the first byte of `from`) every `stride` bytes + const char* from; + const char* to; +}; + +const std::vector& Cases() { + static const std::vector cases = { + // Small expansion (to_len - from_len <= from_len): no scan, upper bound. + {"small/dense expand a->ab", 256, 1, "a", "ab"}, + {"small/sparse expand a->ab", 256, 64, "a", "ab"}, + {"medium/dense expand a->ab", 64 * 1024, 1, "a", "ab"}, + {"medium/sparse expand a->ab", 64 * 1024, 64, "a", "ab"}, + {"large/dense expand a->ab", 4 * 1024 * 1024, 1, "a", "ab"}, + {"large/sparse expand a->ab", 4 * 1024 * 1024, 64, "a", "ab"}, + // Big expansion (to_len - from_len > from_len): falls back to the scan. + {"large/dense bigexp a->abcd", 4 * 1024 * 1024, 1, "a", "abcd"}, + {"large/sparse bigexp a->abcd", 4 * 1024 * 1024, 64, "a", "abcd"}, + // Shrink (to_len <= from_len): no scan. + {"large/dense shrink ab->a", 4 * 1024 * 1024, 2, "ab", "a"}, + }; + return cases; +} + +// Builds a `len`-byte string with `match` once every `stride` bytes. +std::string MakeText(int64_t len, int stride, char match, char filler) { + std::string s(static_cast(len), filler); + for (int64_t i = 0; i < len; i += stride) { + s[static_cast(i)] = match; + } + return s; +} + +// Exact output size, so the "old" arm gets a buffer large enough to complete. +int32_t ExactCapacity(const std::string& text, const char* from, int flen, int olen) { + int64_t matches = 0; + auto tlen = static_cast(text.size()); + if (flen > 0 && flen <= tlen) { + for (int32_t i = 0; i <= tlen - flen;) { + if (memcmp(text.data() + i, from, flen) == 0) { + ++matches; + i += flen; + } else { + ++i; + } + } + } + return static_cast(tlen + matches * (olen - flen)); +} + +void BM_ReplaceNew(benchmark::State& state) { + const ReplaceCase& c = Cases()[state.range(0)]; + auto flen = static_cast(strlen(c.from)); + auto olen = static_cast(strlen(c.to)); + std::string text = MakeText(c.text_len, c.stride, c.from[0], 'x'); + auto tlen = static_cast(text.size()); + ExecutionContext ctx; + auto ctx_ptr = reinterpret_cast(&ctx); + + // One warm-up call doubling as a correctness guard. + int32_t out_len = 0; + replace_utf8_utf8_utf8(ctx_ptr, text.data(), tlen, c.from, flen, c.to, olen, &out_len); + if (ctx.has_error()) { + state.SkipWithError(ctx.get_error().c_str()); + return; + } + + for (auto _ : state) { + ctx.Reset(); + const char* out = replace_utf8_utf8_utf8(ctx_ptr, text.data(), tlen, c.from, flen, + c.to, olen, &out_len); + benchmark::DoNotOptimize(out); + benchmark::DoNotOptimize(out_len); + } + state.SetBytesProcessed(state.iterations() * tlen); + state.SetLabel(c.name); +} + +void BM_ReplaceOld(benchmark::State& state) { + const ReplaceCase& c = Cases()[state.range(0)]; + auto flen = static_cast(strlen(c.from)); + auto olen = static_cast(strlen(c.to)); + std::string text = MakeText(c.text_len, c.stride, c.from[0], 'x'); + auto tlen = static_cast(text.size()); + int32_t capacity = ExactCapacity(text, c.from, flen, olen); + ExecutionContext ctx; + auto ctx_ptr = reinterpret_cast(&ctx); + + int32_t out_len = 0; + replace_with_max_len_utf8_utf8_utf8(ctx_ptr, text.data(), tlen, c.from, flen, c.to, + olen, capacity, &out_len); + if (ctx.has_error()) { + state.SkipWithError(ctx.get_error().c_str()); + return; + } + + for (auto _ : state) { + ctx.Reset(); + const char* out = replace_with_max_len_utf8_utf8_utf8( + ctx_ptr, text.data(), tlen, c.from, flen, c.to, olen, capacity, &out_len); + benchmark::DoNotOptimize(out); + benchmark::DoNotOptimize(out_len); + } + state.SetBytesProcessed(state.iterations() * tlen); + state.SetLabel(c.name); +} + +} // namespace + +BENCHMARK(BM_ReplaceNew) + ->DenseRange(0, static_cast(Cases().size()) - 1) + ->Unit(benchmark::kMicrosecond); +BENCHMARK(BM_ReplaceOld) + ->DenseRange(0, static_cast(Cases().size()) - 1) + ->Unit(benchmark::kMicrosecond); + +} // namespace gandiva