Skip to content

Commit 7f1bb41

Browse files
lriggslriggs
committed
apacheGH-50111: [C++][Gandiva] Improve function error messages (apache#50112)
Gandiva's runtime errors are surfaced to SQL users via `ExecutionContext::set_error_msg`. An audit of all ~90 call sites in `cpp/src/gandiva/` turned up two recurring problems: 1. **No SQL function name in the message.** Users see `"divide by zero error"` or `"Output buffer length can't be negative"` with no indication of which SQL function produced it, making errors hard to localize in long queries. 2. **The offending value is not echoed.** Many messages reject a value (invalid weekday, bad boolean string, out-of-range index, …) without telling the user what was supplied. A good runtime error should answer four questions: *which function*, *what went wrong*, *what value triggered it*, *what's the valid range*. This PR moves the highest-impact messages toward that bar. All edits follow the same pattern: ``` <SQL_FUNCTION_NAME>: <what failed>; <offending value>; <valid range or hint> ``` Where the old message contained a load-bearing substring (e.g. `divide by zero error`, `Output buffer length can't be negative`), the substring is preserved so existing substring-based tests still match. [cpp/src/gandiva/regex_functions_holder.cc:239-247](cpp/src/gandiva/regex_functions_holder.cc#L239-L247) — the message `"Index to extract out of range"` now reads `"REGEXP_EXTRACT: invalid group_index '<N>'; must be between 0 and <max> (the number of capture groups in the pattern)"`, matching the level of detail of the Java path. Eight call sites in `arithmetic_ops.cc`, `decimal_ops.cc`, and `extended_math_ops.cc` (covering `DIVIDE`, `DIV`, `MOD`, `PMOD`, decimal `Divide`/`Mod`, `LOG`) now prefix the SQL function name while preserving the original `divide by zero error` substring. `mod_float64_float64` additionally echoes the dividend. The following sites now include both the SQL function name and the offending value: - `CAST_BIT` — echoes the invalid input string, lists expected values - `CAST_VARCHAR` / `CAST_VARBINARY` length checks — echoes requested length - `REPEAT` — echoes count (and on overflow: count × input length) - `CONVERT_REPLACE_INVALID_FROM_UTF8` — echoes byte count - `LOCATE` — echoes start position - `FACTORIAL` — echoes input value (both negative and >20 paths) - `NEGATIVE` (integer overflow on `INT_MIN`, and `negative_daytimeinterval` out-of-bounds) - `CRC32` — echoes input length - `BASE64` / `UNBASE64` — echoes input length - `AES_ENCRYPT` / `AES_DECRYPT` — echoes data length on negative input; `AES_ENCRYPT` OOM message also rewritten to name the function clearly - `NEXT_DAY` — echoes the unrecognized weekday string, lists expected values - `CAST_INTERVAL_YEAR` — echoes the overflowing source value - `CAST_*_FROM_HEX` (3 error paths in the macro) — echoes the input hex string - `REPLACE` — buffer-overflow message prefixed with `REPLACE:` - Strict `EXPECT_EQ(error, "exact string")` assertions converted to `HasSubstr` / `.find()` so the test contract is the SQL function name plus a stable phrase, not the exact wording. - Updated assertions where the new message no longer contains the old free-text fragment (e.g. `"Factorial of negative"` → `HasSubstr("FACTORIAL") + HasSubstr("non-negative")`). - Files touched: `arithmetic_ops_test.cc`, `decimal_ops_test.cc`, `extended_math_ops_test.cc`, `time_test.cc`, `gdv_function_stubs_test.cc`. ```bash cd cpp/debug cmake --build . --target gandiva_shared gandiva-precompiled-test gandiva-internals-test gandiva-projector-test -j4 ./debug/gandiva-precompiled-test # 130/130 pass ./debug/gandiva-internals-test # 156/156 pass ./debug/gandiva-projector-test # 218/218 pass ``` All affected error paths are exercised by the existing unit tests — that is how each site was located in the audit. Yes, improved error messages. ``` -- before SELECT REGEXP_EXTRACT('100-500', '(\d+)-(\d+)', -1); -- FUNCTION ERROR: Index to extract out of range -- after -- FUNCTION ERROR: REGEXP_EXTRACT: invalid group_index '-1'; -- must be between 0 and 2 -- (the number of capture groups in the pattern) ``` ``` -- before SELECT CAST('maybe' AS BOOLEAN); -- FUNCTION ERROR: Invalid value for boolean. -- after -- FUNCTION ERROR: CAST_BIT: Invalid value for boolean: 'maybe' -- (expected 0, 1, true, false; case-insensitive) ``` ``` -- before SELECT NEXT_DAY(TIMESTAMP '2025-01-01 00:00:00', 'frusday'); -- FUNCTION ERROR: The weekday in this entry is invalid -- after -- FUNCTION ERROR: NEXT_DAY: 'frusday' is not a recognized weekday -- (expected MON|TUE|WED|THU|FRI|SAT|SUN) ``` * GitHub Issue: apache#50111 Authored-by: logan.riggs@gmail.com <logan.riggs@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent 689e9da commit 7f1bb41

12 files changed

Lines changed: 373 additions & 206 deletions

‎cpp/src/gandiva/gdv_function_stubs.cc‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include <algorithm>
2323
#include <boost/crc.hpp>
24+
#include <cstdio>
2425
#include <sstream>
2526
#include <string>
2627
#include <vector>
@@ -192,7 +193,10 @@ int32_t gdv_fn_populate_varlen_vector(int64_t context_ptr, int8_t* data_ptr,
192193
GANDIVA_EXPORT \
193194
int64_t gdv_fn_crc_32_##TYPE(int64_t ctx, const char* input, int32_t input_len) { \
194195
if (input_len < 0) { \
195-
gdv_fn_context_set_error_msg(ctx, "Input length can't be negative"); \
196+
char err_msg[96]; \
197+
snprintf(err_msg, sizeof(err_msg), \
198+
"CRC32: Input length can't be negative, got %d", input_len); \
199+
gdv_fn_context_set_error_msg(ctx, err_msg); \
196200
return 0; \
197201
} \
198202
boost::crc_32_type result; \
@@ -241,7 +245,10 @@ GANDIVA_EXPORT
241245
const char* gdv_fn_base64_encode_binary(int64_t context, const char* in, int32_t in_len,
242246
int32_t* out_len) {
243247
if (in_len < 0) {
244-
gdv_fn_context_set_error_msg(context, "Buffer length cannot be negative");
248+
char err_msg[96];
249+
snprintf(err_msg, sizeof(err_msg),
250+
"BASE64: input length must be non-negative, got %d", in_len);
251+
gdv_fn_context_set_error_msg(context, err_msg);
245252
*out_len = 0;
246253
return "";
247254
}
@@ -268,7 +275,10 @@ GANDIVA_EXPORT
268275
const char* gdv_fn_base64_decode_utf8(int64_t context, const char* in, int32_t in_len,
269276
int32_t* out_len) {
270277
if (in_len < 0) {
271-
gdv_fn_context_set_error_msg(context, "Buffer length cannot be negative");
278+
char err_msg[96];
279+
snprintf(err_msg, sizeof(err_msg),
280+
"UNBASE64: input length must be non-negative, got %d", in_len);
281+
gdv_fn_context_set_error_msg(context, err_msg);
272282
*out_len = 0;
273283
return "";
274284
}

‎cpp/src/gandiva/gdv_function_stubs_test.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ TEST(TestGdvFnStubs, TestBase64Encode) {
127127
value = gdv_fn_base64_encode_binary(ctx_ptr, "test", -5, &out_len);
128128
out_value = std::string(value, out_len);
129129
EXPECT_EQ(out_value, "");
130-
EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("Buffer length cannot be negative"));
130+
EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("BASE64"));
131+
EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("non-negative"));
131132
ctx.Reset();
132133
}
133134

@@ -156,7 +157,8 @@ TEST(TestGdvFnStubs, TestBase64Decode) {
156157
value = gdv_fn_base64_decode_utf8(ctx_ptr, "test", -5, &out_len);
157158
out_value = std::string(value, out_len);
158159
EXPECT_EQ(out_value, "");
159-
EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("Buffer length cannot be negative"));
160+
EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("UNBASE64"));
161+
EXPECT_THAT(ctx.get_error(), ::testing::HasSubstr("non-negative"));
160162
ctx.Reset();
161163
}
162164

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

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
#include <cinttypes>
1819
#include <cmath>
1920
#include <cstdint>
21+
#include <cstdio>
2022
#include "arrow/util/basic_decimal.h"
2123

2224
extern "C" {
@@ -65,7 +67,7 @@ extern "C" {
6567
gdv_##OUT_TYPE NAME##_##IN_TYPE1##_##IN_TYPE2(int64_t context, gdv_##IN_TYPE1 left, \
6668
gdv_##IN_TYPE2 right) { \
6769
if (right == static_cast<gdv_##IN_TYPE2>(0)) { \
68-
gdv_fn_context_set_error_msg(context, "divide by zero error"); \
70+
gdv_fn_context_set_error_msg(context, "PMOD: divide by zero error"); \
6971
return static_cast<gdv_##IN_TYPE1>(0); \
7072
} \
7173
double mod = fmod(static_cast<double>(left), static_cast<double>(right)); \
@@ -109,7 +111,8 @@ PMOD_OP(pmod, float64, float64, float64)
109111

110112
gdv_float64 mod_float64_float64(int64_t context, gdv_float64 x, gdv_float64 y) {
111113
if (y == 0.0) {
112-
const char* err_msg = "divide by zero error";
114+
char err_msg[96];
115+
snprintf(err_msg, sizeof(err_msg), "MOD: divide by zero error (dividend: %g)", x);
113116
gdv_fn_context_set_error_msg(context, err_msg);
114117
return 0.0;
115118
}
@@ -351,7 +354,7 @@ NUMERIC_BOOL_DATE_FUNCTION(IS_NOT_DISTINCT_FROM)
351354
FORCE_INLINE \
352355
gdv_##TYPE divide_##TYPE##_##TYPE(gdv_int64 context, gdv_##TYPE in1, gdv_##TYPE in2) { \
353356
if (in2 == 0) { \
354-
const char* err_msg = "divide by zero error"; \
357+
const char* err_msg = "DIVIDE: divide by zero error"; \
355358
gdv_fn_context_set_error_msg(context, err_msg); \
356359
return 0; \
357360
} \
@@ -376,14 +379,19 @@ NUMERIC_FUNCTION(POSITIVE)
376379

377380
NUMERIC_FUNCTION_FOR_REAL(NEGATIVE)
378381

379-
#define NEGATIVE_INTEGER(TYPE, SIZE) \
380-
FORCE_INLINE \
381-
gdv_##TYPE negative_##TYPE(gdv_int64 context, gdv_##TYPE in) { \
382-
if (in <= INT##SIZE##_MIN) { \
383-
gdv_fn_context_set_error_msg(context, "Overflow in negative execution"); \
384-
return 0; \
385-
} \
386-
return -1 * in; \
382+
#define NEGATIVE_INTEGER(TYPE, SIZE) \
383+
FORCE_INLINE \
384+
gdv_##TYPE negative_##TYPE(gdv_int64 context, gdv_##TYPE in) { \
385+
if (in <= INT##SIZE##_MIN) { \
386+
char err_msg[96]; \
387+
snprintf(err_msg, sizeof(err_msg), \
388+
"NEGATIVE: Overflow in negative execution " \
389+
"(cannot negate INT" #SIZE "_MIN: %" PRId64 ")", \
390+
static_cast<int64_t>(in)); \
391+
gdv_fn_context_set_error_msg(context, err_msg); \
392+
return 0; \
393+
} \
394+
return -1 * in; \
387395
}
388396

389397
NEGATIVE_INTEGER(int32, 32)
@@ -396,8 +404,12 @@ const int64_t INT_MIN_TO_NEGATIVE_INTERVAL_DAY_TIME = -9223372030412324863;
396404
gdv_int64 negative_daytimeinterval(gdv_int64 context, gdv_day_time_interval interval) {
397405
if (interval > INT_MAX_TO_NEGATIVE_INTERVAL_DAY_TIME ||
398406
interval < INT_MIN_TO_NEGATIVE_INTERVAL_DAY_TIME) {
399-
gdv_fn_context_set_error_msg(
400-
context, "Interval day time is out of boundaries for the negative function");
407+
char err_msg[128];
408+
snprintf(err_msg, sizeof(err_msg),
409+
"NEGATIVE: Interval day time is out of boundaries for the negative "
410+
"function (value: %" PRId64 ")",
411+
static_cast<int64_t>(interval));
412+
gdv_fn_context_set_error_msg(context, err_msg);
401413
return 0;
402414
}
403415

@@ -430,7 +442,7 @@ void negative_decimal(gdv_int64 context, int64_t high_bits, uint64_t low_bits,
430442
FORCE_INLINE \
431443
gdv_##TYPE div_##TYPE##_##TYPE(gdv_int64 context, gdv_##TYPE in1, gdv_##TYPE in2) { \
432444
if (in2 == 0) { \
433-
const char* err_msg = "divide by zero error"; \
445+
const char* err_msg = "DIV: divide by zero error"; \
434446
gdv_fn_context_set_error_msg(context, err_msg); \
435447
return 0; \
436448
} \
@@ -448,7 +460,7 @@ DIV(uint64)
448460
FORCE_INLINE \
449461
gdv_##TYPE div_##TYPE##_##TYPE(gdv_int64 context, gdv_##TYPE in1, gdv_##TYPE in2) { \
450462
if (in2 == 0) { \
451-
const char* err_msg = "divide by zero error"; \
463+
const char* err_msg = "DIV: divide by zero error"; \
452464
gdv_fn_context_set_error_msg(context, err_msg); \
453465
return 0; \
454466
} \

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ TEST(TestArithmeticOps, TestPmod) {
5252

5353
EXPECT_EQ(pmod_int64_int64(ctx, 3, 0), 0);
5454
EXPECT_TRUE(context.has_error());
55-
EXPECT_EQ(context.get_error(), "divide by zero error");
55+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
5656
context.Reset();
5757
}
5858

@@ -65,7 +65,7 @@ TEST(TestArithmeticOps, TestMod) {
6565
EXPECT_DOUBLE_EQ(mod_float64_float64(reinterpret_cast<gdv_int64>(&context), 2.5, 0.0),
6666
0.0);
6767
EXPECT_TRUE(context.has_error());
68-
EXPECT_EQ(context.get_error(), "divide by zero error");
68+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
6969

7070
context.Reset();
7171
EXPECT_NEAR(mod_float64_float64(reinterpret_cast<gdv_int64>(&context), 2.5, 1.2), 0.1,
@@ -219,8 +219,9 @@ TEST(TestArithmeticOps, TestNegativeIntervalTypes) {
219219

220220
result = negative_daytimeinterval(ctx_ptr, INT64_MAX);
221221
EXPECT_EQ(ctx.has_error(), true);
222-
EXPECT_EQ(ctx.get_error(),
223-
"Interval day time is out of boundaries for the negative function");
222+
EXPECT_THAT(ctx.get_error(),
223+
::testing::HasSubstr(
224+
"Interval day time is out of boundaries for the negative function"));
224225
ctx.Reset();
225226

226227
const int64_t INT_MIN_TO_NEGATIVE_INTERVAL_DAY_TIME = -9223372030412324863;
@@ -229,8 +230,9 @@ TEST(TestArithmeticOps, TestNegativeIntervalTypes) {
229230

230231
result = negative_daytimeinterval(ctx_ptr, INT64_MIN);
231232
EXPECT_EQ(ctx.has_error(), true);
232-
EXPECT_EQ(ctx.get_error(),
233-
"Interval day time is out of boundaries for the negative function");
233+
EXPECT_THAT(ctx.get_error(),
234+
::testing::HasSubstr(
235+
"Interval day time is out of boundaries for the negative function"));
234236
ctx.Reset();
235237

236238
// Month interval
@@ -251,7 +253,7 @@ TEST(TestArithmeticOps, TestDivide) {
251253
gandiva::ExecutionContext context;
252254
EXPECT_EQ(divide_int64_int64(reinterpret_cast<gdv_int64>(&context), 10, 0), 0);
253255
EXPECT_EQ(context.has_error(), true);
254-
EXPECT_EQ(context.get_error(), "divide by zero error");
256+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
255257

256258
context.Reset();
257259
EXPECT_EQ(divide_int64_int64(reinterpret_cast<gdv_int64>(&context), 10, 2), 5);
@@ -262,7 +264,7 @@ TEST(TestArithmeticOps, TestDiv) {
262264
gandiva::ExecutionContext context;
263265
EXPECT_EQ(div_int64_int64(reinterpret_cast<gdv_int64>(&context), 101, 0), 0);
264266
EXPECT_EQ(context.has_error(), true);
265-
EXPECT_EQ(context.get_error(), "divide by zero error");
267+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
266268
context.Reset();
267269

268270
EXPECT_EQ(div_int64_int64(reinterpret_cast<gdv_int64>(&context), 101, 111), 0);
@@ -278,7 +280,7 @@ TEST(TestArithmeticOps, TestDiv) {
278280
div_float64_float64(reinterpret_cast<gdv_int64>(&context), 1010.1010, 0.00000),
279281
0.0);
280282
EXPECT_EQ(context.has_error(), true);
281-
EXPECT_EQ(context.get_error(), "divide by zero error");
283+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
282284
context.Reset();
283285

284286
EXPECT_EQ(div_float32_float32(reinterpret_cast<gdv_int64>(&context), 1010.1010f, 2.1f),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ BasicDecimal128 Divide(int64_t context, const BasicDecimalScalar128& x,
351351
const BasicDecimalScalar128& y, int32_t out_precision,
352352
int32_t out_scale, bool* overflow) {
353353
if (y.value() == 0) {
354-
const char* err_msg = "divide by zero error";
354+
const char* err_msg = "DIVIDE: divide by zero error (decimal)";
355355
gdv_fn_context_set_error_msg(context, err_msg);
356356
return 0;
357357
}
@@ -396,7 +396,7 @@ BasicDecimal128 Mod(int64_t context, const BasicDecimalScalar128& x,
396396
const BasicDecimalScalar128& y, int32_t out_precision,
397397
int32_t out_scale, bool* overflow) {
398398
if (y.value() == 0) {
399-
const char* err_msg = "divide by zero error";
399+
const char* err_msg = "MOD: divide by zero error (decimal)";
400400
gdv_fn_context_set_error_msg(context, err_msg);
401401
return 0;
402402
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
#include <gmock/gmock.h>
1819
#include <gtest/gtest.h>
1920
#include <algorithm>
2021
#include <limits>
@@ -455,7 +456,7 @@ TEST_F(TestDecimalSql, DivideByZero) {
455456
DecimalScalar128{"201", 20, 3}, DecimalScalar128{"0", 20, 2},
456457
result_precision, result_scale, &overflow);
457458
EXPECT_TRUE(context.has_error());
458-
EXPECT_EQ(context.get_error(), "divide by zero error");
459+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
459460

460461
// divide-by-nonzero should not cause an error.
461462
context.Reset();
@@ -472,7 +473,7 @@ TEST_F(TestDecimalSql, DivideByZero) {
472473
DecimalScalar128{"0", 20, 2}, result_precision, result_scale,
473474
&overflow);
474475
EXPECT_TRUE(context.has_error());
475-
EXPECT_EQ(context.get_error(), "divide by zero error");
476+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"));
476477

477478
// mod-by-nonzero should not cause an error.
478479
context.Reset();

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
extern "C" {
2626

27+
#include <inttypes.h>
2728
#include <math.h>
2829
#include <stdio.h>
2930
#include <stdlib.h>
@@ -80,7 +81,7 @@ ENUMERIC_TYPES_UNARY(LOG10, float64)
8081

8182
FORCE_INLINE
8283
void set_error_for_logbase(int64_t execution_context, double base) {
83-
const char* prefix = "divide by zero error with log of base";
84+
const char* prefix = "LOG: divide by zero error with log of base";
8485
int size = static_cast<int>(strlen(prefix)) + 64;
8586
char* error = reinterpret_cast<char*>(malloc(size));
8687
snprintf(error, size, "%s %f", prefix, base);
@@ -251,20 +252,28 @@ static const int64_t kFactorialLookupTable[] = {1,
251252
121645100408832000,
252253
2432902008176640000};
253254

254-
#define FACTORIAL(IN_TYPE) \
255-
FORCE_INLINE \
256-
gdv_int64 factorial_##IN_TYPE(gdv_int64 ctx, gdv_##IN_TYPE value) { \
257-
if (value < 0) { \
258-
gdv_fn_context_set_error_msg(ctx, "Factorial of negative number not exist!"); \
259-
return 0; \
260-
} \
261-
/* For numbers greater than 20 causes an overflow. */ \
262-
if (value > 20) { \
263-
gdv_fn_context_set_error_msg(ctx, "Numbers greater than 20 cause overflow!"); \
264-
return 0; \
265-
} \
266-
\
267-
return kFactorialLookupTable[static_cast<int32_t>(value)]; \
255+
#define FACTORIAL(IN_TYPE) \
256+
FORCE_INLINE \
257+
gdv_int64 factorial_##IN_TYPE(gdv_int64 ctx, gdv_##IN_TYPE value) { \
258+
if (value < 0) { \
259+
char err_msg[96]; \
260+
snprintf(err_msg, sizeof(err_msg), \
261+
"FACTORIAL: input must be non-negative, got %" PRId64, \
262+
static_cast<int64_t>(value)); \
263+
gdv_fn_context_set_error_msg(ctx, err_msg); \
264+
return 0; \
265+
} \
266+
/* For numbers greater than 20 causes an overflow. */ \
267+
if (value > 20) { \
268+
char err_msg[96]; \
269+
snprintf(err_msg, sizeof(err_msg), \
270+
"FACTORIAL: input %" PRId64 " exceeds maximum 20 (would overflow int64)", \
271+
static_cast<int64_t>(value)); \
272+
gdv_fn_context_set_error_msg(ctx, err_msg); \
273+
return 0; \
274+
} \
275+
\
276+
return kFactorialLookupTable[static_cast<int32_t>(value)]; \
268277
}
269278

270279
FACTORIAL(int32)

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
# define M_PI 3.14159265358979323846
2020
#endif
2121

22+
#include <gmock/gmock.h>
2223
#include <gtest/gtest.h>
2324

2425
#include <cmath>
@@ -61,11 +62,12 @@ TEST(TestExtendedMathOps, TestFactorial) {
6162
}
6263

6364
factorial_int32(ctx, 21);
64-
EXPECT_TRUE(context.get_error().find("overflow") != std::string::npos);
65+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("overflow"));
6566
context.Reset();
6667

6768
factorial_int32(ctx, -5);
68-
EXPECT_TRUE(context.get_error().find("Factorial of negative") != std::string::npos);
69+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("FACTORIAL"));
70+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("non-negative"));
6971
context.Reset();
7072

7173
for (int64_t i = 0; i <= 20; ++i) {
@@ -79,11 +81,12 @@ TEST(TestExtendedMathOps, TestFactorial) {
7981
}
8082

8183
factorial_int64(ctx, 21);
82-
EXPECT_TRUE(context.get_error().find("overflow") != std::string::npos);
84+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("overflow"));
8385
context.Reset();
8486

8587
factorial_int64(ctx, -5);
86-
EXPECT_TRUE(context.get_error().find("Factorial of negative") != std::string::npos);
88+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("FACTORIAL"));
89+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("non-negative"));
8790
context.Reset();
8891
}
8992

@@ -125,7 +128,7 @@ TEST(TestExtendedMathOps, TestLogWithBase) {
125128
log_int32_int32(reinterpret_cast<gdv_int64>(&context), 1 /*base*/, 10 /*value*/);
126129
VerifyFuzzyEquals(out, 0);
127130
EXPECT_EQ(context.has_error(), true);
128-
EXPECT_TRUE(context.get_error().find("divide by zero error") != std::string::npos)
131+
EXPECT_THAT(context.get_error(), ::testing::HasSubstr("divide by zero error"))
129132
<< context.get_error();
130133

131134
gandiva::ExecutionContext context1;

0 commit comments

Comments
 (0)