diff --git a/cpp/src/gandiva/expr_validator.cc b/cpp/src/gandiva/expr_validator.cc index 27b27fbe25a..ff46de2283f 100644 --- a/cpp/src/gandiva/expr_validator.cc +++ b/cpp/src/gandiva/expr_validator.cc @@ -24,9 +24,14 @@ namespace gandiva { namespace { -inline Status ValidateInExpression(size_t number_of_values, - const DataTypePtr& in_expr_return_type, - const DataTypePtr& types_of_values) { +inline Status ValidateInExpression(const NodePtr& eval_expr, size_t number_of_values, + const DataTypePtr& types_of_values, + ExprValidator& validator) { + ARROW_RETURN_IF(eval_expr == nullptr || eval_expr->return_type() == nullptr, + Status::ExpressionValidationError( + "IN evaluation expression and type cannot be null")); + ARROW_RETURN_NOT_OK(eval_expr->Accept(validator)); + const auto& in_expr_return_type = eval_expr->return_type(); ARROW_RETURN_IF(number_of_values == 0, Status::ExpressionValidationError( "IN Expression needs a non-empty constant list to match.")); @@ -41,9 +46,11 @@ inline Status ValidateInExpression(size_t number_of_values, template inline Status ValidateInExpression(const InExpressionNode& node, - const DataTypePtr& types_of_values_default) { - return ValidateInExpression(node.values().size(), node.eval_expr()->return_type(), - node.type() ? node.type() : types_of_values_default); + const DataTypePtr& types_of_values_default, + ExprValidator& validator) { + return ValidateInExpression(node.eval_expr(), node.values().size(), + node.type() ? node.type() : types_of_values_default, + validator); } } // namespace @@ -173,26 +180,27 @@ Status ExprValidator::Visit(const BooleanNode& node) { * 2. Expression returns of the same type as the constants. */ Status ExprValidator::Visit(const InExpressionNode& node) { - return ValidateInExpression(node, arrow::int32()); + return ValidateInExpression(node, arrow::int32(), *this); } Status ExprValidator::Visit(const InExpressionNode& node) { - return ValidateInExpression(node, arrow::int64()); + return ValidateInExpression(node, arrow::int64(), *this); } Status ExprValidator::Visit(const InExpressionNode& node) { - return ValidateInExpression(node, arrow::float32()); + return ValidateInExpression(node, arrow::float32(), *this); } Status ExprValidator::Visit(const InExpressionNode& node) { - return ValidateInExpression(node, arrow::float64()); + return ValidateInExpression(node, arrow::float64(), *this); } Status ExprValidator::Visit(const InExpressionNode& node) { - return ValidateInExpression(node.values().size(), node.eval_expr()->return_type(), - arrow::decimal128(node.get_precision(), node.get_scale())); + return ValidateInExpression(node.eval_expr(), node.values().size(), + arrow::decimal128(node.get_precision(), node.get_scale()), + *this); } Status ExprValidator::Visit(const InExpressionNode& node) { - return ValidateInExpression(node, arrow::utf8()); + return ValidateInExpression(node, arrow::utf8(), *this); } } // namespace gandiva diff --git a/cpp/src/gandiva/node.h b/cpp/src/gandiva/node.h index 858c6570489..2ea0644edda 100644 --- a/cpp/src/gandiva/node.h +++ b/cpp/src/gandiva/node.h @@ -251,7 +251,7 @@ class InExpressionNode : public Node { std::string ToString() const override { std::stringstream ss; - ss << eval_expr_->ToString() << " IN ("; + ss << (eval_expr_ ? eval_expr_->ToString() : std::string("null")) << " IN ("; bool add_comma = false; for (auto& value : values_) { if (add_comma) { @@ -295,7 +295,7 @@ class InExpressionNode : public Node { std::string ToString() const override { std::stringstream ss; - ss << eval_expr_->ToString() << " IN ("; + ss << (eval_expr_ ? eval_expr_->ToString() : std::string("null")) << " IN ("; bool add_comma = false; for (auto& value : values_) { if (add_comma) { diff --git a/cpp/src/gandiva/tests/projector_build_validation_test.cc b/cpp/src/gandiva/tests/projector_build_validation_test.cc index 1ed4c77a074..3e8cc94fc2a 100644 --- a/cpp/src/gandiva/tests/projector_build_validation_test.cc +++ b/cpp/src/gandiva/tests/projector_build_validation_test.cc @@ -17,6 +17,7 @@ #include #include "arrow/memory_pool.h" +#include "gandiva/filter.h" #include "gandiva/projector.h" #include "gandiva/tests/test_util.h" #include "gandiva/tree_expr_builder.h" @@ -284,4 +285,93 @@ TEST_F(ValidationTestProjector, TestAndBooleanArgType) { EXPECT_TRUE(status.IsExpressionValidationError()); } +TEST_F(ValidationTestProjector, TestInExpressionValidatesEvalExpression) { + auto schema = arrow::schema({field("f0", int32())}); + auto decimal_type = arrow::decimal128(10, 0); + + // An IF with an int32 condition is invalid, but its return type matches the IN values. + auto invalid_if = [](const NodePtr& value, const DataTypePtr& type) { + return TreeExprBuilder::MakeIf(TreeExprBuilder::MakeLiteral(int32_t{1}), value, value, + type); + }; + std::unordered_set decimals{DecimalScalar128("1", 10, 0)}; + NodeVector invalid = { + TreeExprBuilder::MakeInExpressionInt32( + invalid_if(TreeExprBuilder::MakeLiteral(int32_t{1}), int32()), {1}), + TreeExprBuilder::MakeInExpressionInt64( + invalid_if(TreeExprBuilder::MakeLiteral(int64_t{1}), arrow::int64()), {1}), + TreeExprBuilder::MakeInExpressionFloat( + invalid_if(TreeExprBuilder::MakeLiteral(1.0f), float32()), {1.0f}), + TreeExprBuilder::MakeInExpressionDouble( + invalid_if(TreeExprBuilder::MakeLiteral(1.0), arrow::float64()), {1.0}), + TreeExprBuilder::MakeInExpressionString( + invalid_if(TreeExprBuilder::MakeStringLiteral("a"), arrow::utf8()), {"a"}), + TreeExprBuilder::MakeInExpressionBinary( + invalid_if(TreeExprBuilder::MakeBinaryLiteral("a"), arrow::binary()), {"a"}), + TreeExprBuilder::MakeInExpressionDecimal( + invalid_if(TreeExprBuilder::MakeDecimalLiteral(DecimalScalar128("1", 10, 0)), + decimal_type), + decimals)}; + + for (size_t i = 0; i < invalid.size(); ++i) { + SCOPED_TRACE(i); + std::shared_ptr projector; + auto expr = TreeExprBuilder::MakeExpression(invalid[i], field("res", boolean())); + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.IsExpressionValidationError()) << status.ToString(); + + std::shared_ptr filter; + status = Filter::Make(schema, TreeExprBuilder::MakeCondition(invalid[i]), + TestConfiguration(), &filter); + EXPECT_TRUE(status.IsExpressionValidationError()) << status.ToString(); + } +} + +TEST_F(ValidationTestProjector, TestInExpressionNullEvalExpression) { + auto schema = arrow::schema({field("f0", int32())}); + NodeVector invalid = {std::make_shared>( + nullptr, std::unordered_set{1}), + std::make_shared>( + nullptr, std::unordered_set{"a"})}; + + for (size_t i = 0; i < invalid.size(); ++i) { + SCOPED_TRACE(i); + std::shared_ptr projector; + auto expr = TreeExprBuilder::MakeExpression(invalid[i], field("res", boolean())); + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.IsExpressionValidationError()) << status.ToString(); + + std::shared_ptr filter; + status = Filter::Make(schema, TreeExprBuilder::MakeCondition(invalid[i]), + TestConfiguration(), &filter); + EXPECT_TRUE(status.IsExpressionValidationError()) << status.ToString(); + } +} + +TEST_F(ValidationTestProjector, TestInExpressionUntypedEvalExpression) { + auto schema = arrow::schema({field("f0", int32())}); + auto untyped = std::make_shared(nullptr, LiteralHolder(int32_t{1}), false); + auto in = TreeExprBuilder::MakeInExpressionInt32(untyped, {1}); + + std::shared_ptr projector; + auto expr = TreeExprBuilder::MakeExpression(in, field("res", boolean())); + auto status = Projector::Make(schema, {expr}, TestConfiguration(), &projector); + EXPECT_TRUE(status.IsExpressionValidationError()) << status.ToString(); +} + +TEST_F(ValidationTestProjector, TestInExpressionValid) { + auto field0 = field("f0", int32()); + auto schema = arrow::schema({field0}); + auto in = + TreeExprBuilder::MakeInExpressionInt32(TreeExprBuilder::MakeField(field0), {1, 2}); + + std::shared_ptr projector; + auto expr = TreeExprBuilder::MakeExpression(in, field("res", boolean())); + ASSERT_OK(Projector::Make(schema, {expr}, TestConfiguration(), &projector)); + + std::shared_ptr filter; + ASSERT_OK(Filter::Make(schema, TreeExprBuilder::MakeCondition(in), TestConfiguration(), + &filter)); +} + } // namespace gandiva