Skip to content

GH-50111: [C++][Gandiva] Improve function error messages #50112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lriggs wants to merge 3 commits into
base: main
Choose a base branch
from
Open

GH-50111: [C++][Gandiva] Improve function error messages #50112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb79c3c
GH-50111: [C++][Gandiva] Improve function error messages
lriggs Jun 5, 2026
c3be9f0
Clarrify error message and add test for case insensitivity
lriggs Jun 8, 2026
8ab4c2b
Update next day error message.
lriggs Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions cpp/src/gandiva/gdv_function_stubs.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <utf8proc.h>

#include <boost/crc.hpp>
#include <cstdio>
#include <sstream>
#include <string>
#include <vector>
Expand Down Expand Up @@ -189,7 +190,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; \
Expand Down Expand Up @@ -233,7 +237,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 "";
}
Expand All @@ -260,7 +267,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 "";
}
Expand Down Expand Up @@ -343,7 +353,10 @@ const char* gdv_fn_aes_encrypt(int64_t context, const char* data, int32_t data_l
const char* key_data, int32_t key_data_len,
int32_t* out_len) {
if (data_len < 0) {
gdv_fn_context_set_error_msg(context, "Invalid data length to be encrypted");
char err_msg[96];
snprintf(err_msg, sizeof(err_msg),
"AES_ENCRYPT: data length can't be negative, got %d", data_len);
gdv_fn_context_set_error_msg(context, err_msg);
*out_len = 0;
return "";
}
Expand All @@ -363,8 +376,7 @@ const char* gdv_fn_aes_encrypt(int64_t context, const char* data, int32_t data_l
static_cast<int32_t>(arrow::bit_util::RoundUpToPowerOf2(data_len, kAesBlockSize));
char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len));
if (ret == nullptr) {
std::string err_msg =
"Could not allocate memory for returning aes encrypt cypher text";
std::string err_msg = "AES_ENCRYPT: could not allocate memory for ciphertext output";
gdv_fn_context_set_error_msg(context, err_msg.data());
*out_len = 0;
return nullptr;
Expand All @@ -387,7 +399,10 @@ const char* gdv_fn_aes_decrypt(int64_t context, const char* data, int32_t data_l
const char* key_data, int32_t key_data_len,
int32_t* out_len) {
if (data_len < 0) {
gdv_fn_context_set_error_msg(context, "Invalid data length to be decrypted");
char err_msg[96];
snprintf(err_msg, sizeof(err_msg),
"AES_DECRYPT: data length can't be negative, got %d", data_len);
gdv_fn_context_set_error_msg(context, err_msg);
*out_len = 0;
return "";
}
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/gandiva/gdv_function_stubs_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,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();
}

Expand Down Expand Up @@ -153,7 +154,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();
}

Expand Down
42 changes: 27 additions & 15 deletions cpp/src/gandiva/precompiled/arithmetic_ops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
// specific language governing permissions and limitations
// under the License.

#include <cinttypes>
#include <cmath>
#include <cstdint>
#include <cstdio>
#include "arrow/util/basic_decimal.h"

extern "C" {
Expand Down Expand Up @@ -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<gdv_##IN_TYPE2>(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<gdv_##IN_TYPE1>(0); \
} \
double mod = fmod(static_cast<double>(left), static_cast<double>(right)); \
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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; \
} \
Expand All @@ -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<int64_t>(in)); \
gdv_fn_context_set_error_msg(context, err_msg); \
return 0; \
} \
return -1 * in; \
}

NEGATIVE_INTEGER(int32, 32)
Expand All @@ -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<int64_t>(interval));
gdv_fn_context_set_error_msg(context, err_msg);
return 0;
}

Expand Down Expand Up @@ -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; \
} \
Expand All @@ -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; \
} \
Expand Down
20 changes: 11 additions & 9 deletions cpp/src/gandiva/precompiled/arithmetic_ops_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -65,7 +65,7 @@ TEST(TestArithmeticOps, TestMod) {
EXPECT_DOUBLE_EQ(mod_float64_float64(reinterpret_cast<gdv_int64>(&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<gdv_int64>(&context), 2.5, 1.2), 0.1,
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -251,7 +253,7 @@ TEST(TestArithmeticOps, TestDivide) {
gandiva::ExecutionContext context;
EXPECT_EQ(divide_int64_int64(reinterpret_cast<gdv_int64>(&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<gdv_int64>(&context), 10, 2), 5);
Expand All @@ -262,7 +264,7 @@ TEST(TestArithmeticOps, TestDiv) {
gandiva::ExecutionContext context;
EXPECT_EQ(div_int64_int64(reinterpret_cast<gdv_int64>(&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<gdv_int64>(&context), 101, 111), 0);
Expand All @@ -278,7 +280,7 @@ TEST(TestArithmeticOps, TestDiv) {
div_float64_float64(reinterpret_cast<gdv_int64>(&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<gdv_int64>(&context), 1010.1010f, 2.1f),
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/gandiva/precompiled/decimal_ops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/gandiva/precompiled/decimal_ops_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,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_NE(context.get_error().find("divide by zero error"), std::string::npos);

@kou kou Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use EXPECT_THAT(::testing::HasSubstr()) like others?


// divide-by-nonzero should not cause an error.
context.Reset();
Expand All @@ -472,7 +472,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_NE(context.get_error().find("divide by zero error"), std::string::npos);

// mod-by-nonzero should not cause an error.
context.Reset();
Expand Down
39 changes: 24 additions & 15 deletions cpp/src/gandiva/precompiled/extended_math_ops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

extern "C" {

#include <inttypes.h>
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -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<int>(strlen(prefix)) + 64;
char* error = reinterpret_cast<char*>(malloc(size));
snprintf(error, size, "%s %f", prefix, base);
Expand Down Expand Up @@ -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<int32_t>(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<int64_t>(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<int64_t>(value)); \
gdv_fn_context_set_error_msg(ctx, err_msg); \
return 0; \
} \
\
return kFactorialLookupTable[static_cast<int32_t>(value)]; \
}

FACTORIAL(int32)
Expand Down
Loading
Loading