kou commented on code in PR #46709: URL: https://github.com/apache/arrow/pull/46709#discussion_r2127733596
########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); + auto int_literal_multiply = TreeExprBuilder::MakeLiteral((int32_t)10); + auto string_literal = TreeExprBuilder::MakeStringLiteral("foo"); + auto cast_multiply_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal_multiply}, decimal_type_10_0); + auto cast_int_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal}, decimal_type_38_30); + auto cast_string_func = + TreeExprBuilder::MakeFunction("castDECIMAL", {string_literal}, decimal_type_38_30); + auto multiply_func = + TreeExprBuilder::MakeFunction("multiply", {cast_multiply_literal, cast_int_literal}, decimal_type_38_27); + auto equal_func = TreeExprBuilder::MakeFunction("equal", {multiply_func, cast_string_func}, arrow::boolean()); + auto expr = TreeExprBuilder::MakeExpression(equal_func, res_bool); + + std::shared_ptr<Projector> projector; + + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); + + int num_records = 1; + auto invalid_in = MakeArrowArrayUtf8({"1.345"}, {true}); + auto in_batch_1 = arrow::RecordBatch::Make(schema, num_records, {invalid_in}); + + arrow::ArrayVector outputs_1; + status = projector->Evaluate(*in_batch_1, pool_, &outputs_1); + EXPECT_FALSE(status.ok()) << status.message(); Review Comment: ```suggestion ASSERT_OK(projector->Evaluate(*in_batch_1, pool_, &outputs_1)); ``` ########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); + auto int_literal_multiply = TreeExprBuilder::MakeLiteral((int32_t)10); + auto string_literal = TreeExprBuilder::MakeStringLiteral("foo"); + auto cast_multiply_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal_multiply}, decimal_type_10_0); + auto cast_int_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal}, decimal_type_38_30); + auto cast_string_func = + TreeExprBuilder::MakeFunction("castDECIMAL", {string_literal}, decimal_type_38_30); + auto multiply_func = + TreeExprBuilder::MakeFunction("multiply", {cast_multiply_literal, cast_int_literal}, decimal_type_38_27); + auto equal_func = TreeExprBuilder::MakeFunction("equal", {multiply_func, cast_string_func}, arrow::boolean()); + auto expr = TreeExprBuilder::MakeExpression(equal_func, res_bool); + + std::shared_ptr<Projector> projector; + + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); + + int num_records = 1; + auto invalid_in = MakeArrowArrayUtf8({"1.345"}, {true}); + auto in_batch_1 = arrow::RecordBatch::Make(schema, num_records, {invalid_in}); + + arrow::ArrayVector outputs_1; + status = projector->Evaluate(*in_batch_1, pool_, &outputs_1); + EXPECT_FALSE(status.ok()) << status.message(); + EXPECT_NE(status.message().find("not a valid decimal128 number"), std::string::npos); Review Comment: ```suggestion ASSERT_THAT(status.message(), ::testing::HasSubstr("not a valid decimal128 number")); ``` ########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); + auto int_literal_multiply = TreeExprBuilder::MakeLiteral((int32_t)10); + auto string_literal = TreeExprBuilder::MakeStringLiteral("foo"); + auto cast_multiply_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal_multiply}, decimal_type_10_0); + auto cast_int_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal}, decimal_type_38_30); + auto cast_string_func = + TreeExprBuilder::MakeFunction("castDECIMAL", {string_literal}, decimal_type_38_30); + auto multiply_func = + TreeExprBuilder::MakeFunction("multiply", {cast_multiply_literal, cast_int_literal}, decimal_type_38_27); + auto equal_func = TreeExprBuilder::MakeFunction("equal", {multiply_func, cast_string_func}, arrow::boolean()); + auto expr = TreeExprBuilder::MakeExpression(equal_func, res_bool); + + std::shared_ptr<Projector> projector; + + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); Review Comment: ```suggestion ASSERT_OK(Projector::Make(schema, {expr}, TestConfiguration(), &projector)); ``` ########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); + auto int_literal_multiply = TreeExprBuilder::MakeLiteral((int32_t)10); Review Comment: ```suggestion auto int_literal_multiply = TreeExprBuilder::MakeLiteral(static_cast<int32_t>(10)); ``` ########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); Review Comment: ```suggestion auto int_literal = TreeExprBuilder::MakeLiteral(static_cast<int32_t>(100)); ``` ########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); + auto int_literal_multiply = TreeExprBuilder::MakeLiteral((int32_t)10); + auto string_literal = TreeExprBuilder::MakeStringLiteral("foo"); + auto cast_multiply_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal_multiply}, decimal_type_10_0); + auto cast_int_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal}, decimal_type_38_30); + auto cast_string_func = + TreeExprBuilder::MakeFunction("castDECIMAL", {string_literal}, decimal_type_38_30); + auto multiply_func = + TreeExprBuilder::MakeFunction("multiply", {cast_multiply_literal, cast_int_literal}, decimal_type_38_27); + auto equal_func = TreeExprBuilder::MakeFunction("equal", {multiply_func, cast_string_func}, arrow::boolean()); + auto expr = TreeExprBuilder::MakeExpression(equal_func, res_bool); + + std::shared_ptr<Projector> projector; + + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.ok()) << status.message(); + + int num_records = 1; + auto invalid_in = MakeArrowArrayUtf8({"1.345"}, {true}); + auto in_batch_1 = arrow::RecordBatch::Make(schema, num_records, {invalid_in}); Review Comment: ```suggestion ASSERT_OK_AND_ASSIGN(auto in_batch_1, arrow::RecordBatch::Make(schema, num_records, {invalid_in})); ``` BTW, why do we need `_1` suffix here? It seems that there is only one record batch. ########## cpp/src/gandiva/tests/decimal_test.cc: ########## @@ -1237,4 +1237,42 @@ TEST_F(TestDecimal, TestSha) { EXPECT_NE(value_at_position, response->GetScalar(i - 1).ValueOrDie()->ToString()); } } + +TEST_F(TestDecimal, TestCastDecimalVarCharInvalidInputInvalidOutput) { + auto decimal_type_10_0 = std::make_shared<arrow::Decimal128Type>(10, 0); + auto decimal_type_38_30 = std::make_shared<arrow::Decimal128Type>(38, 30); + auto decimal_type_38_27 = std::make_shared<arrow::Decimal128Type>(38, 27); + + auto field_str = field("in_str", utf8()); + auto schema = arrow::schema({field_str}); + auto res_bool = field("res_bool", arrow::boolean()); + + auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)100); + auto int_literal_multiply = TreeExprBuilder::MakeLiteral((int32_t)10); + auto string_literal = TreeExprBuilder::MakeStringLiteral("foo"); + auto cast_multiply_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal_multiply}, decimal_type_10_0); + auto cast_int_literal = + TreeExprBuilder::MakeFunction("castDECIMAL", {int_literal}, decimal_type_38_30); + auto cast_string_func = + TreeExprBuilder::MakeFunction("castDECIMAL", {string_literal}, decimal_type_38_30); + auto multiply_func = + TreeExprBuilder::MakeFunction("multiply", {cast_multiply_literal, cast_int_literal}, decimal_type_38_27); + auto equal_func = TreeExprBuilder::MakeFunction("equal", {multiply_func, cast_string_func}, arrow::boolean()); + auto expr = TreeExprBuilder::MakeExpression(equal_func, res_bool); Review Comment: Do we need them? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org