diff --git a/src/iceberg/expression/binder.cc b/src/iceberg/expression/binder.cc index 43c3ebcdf..3b053cdc6 100644 --- a/src/iceberg/expression/binder.cc +++ b/src/iceberg/expression/binder.cc @@ -19,6 +19,8 @@ #include "iceberg/expression/binder.h" +#include "iceberg/util/macros.h" + namespace iceberg { Binder::Binder(const Schema& schema, bool case_sensitive) @@ -89,11 +91,13 @@ Result IsBoundVisitor::AlwaysFalse() { return true; } Result IsBoundVisitor::Not(bool child_result) { return child_result; } Result IsBoundVisitor::And(bool left_result, bool right_result) { - return left_result && right_result; + ICEBERG_PRECHECK(left_result == right_result, "Found partially bound expression"); + return left_result; } Result IsBoundVisitor::Or(bool left_result, bool right_result) { - return left_result && right_result; + ICEBERG_PRECHECK(left_result == right_result, "Found partially bound expression"); + return left_result; } Result IsBoundVisitor::Predicate(const std::shared_ptr& pred) { diff --git a/src/iceberg/test/expression_visitor_test.cc b/src/iceberg/test/expression_visitor_test.cc index f2bbe70ea..0bf6585dd 100644 --- a/src/iceberg/test/expression_visitor_test.cc +++ b/src/iceberg/test/expression_visitor_test.cc @@ -332,14 +332,14 @@ TEST_F(IsBoundVisitorTest, AndWithBoundChildren) { } TEST_F(IsBoundVisitorTest, AndWithUnboundChild) { - // AND with any unbound child should return false + // AND with mixed bound/unbound children should error auto bound_pred = Expressions::Equal("name", Literal::String("Alice")); ICEBERG_UNWRAP_OR_FAIL(auto pred1, Bind(bound_pred)); auto pred2 = Expressions::Equal("age", Literal::Int(25)); // unbound auto mixed_and = Expressions::And(pred1, pred2); - ICEBERG_UNWRAP_OR_FAIL(auto is_bound, IsBoundVisitor::IsBound(mixed_and)); - EXPECT_FALSE(is_bound); + auto result = IsBoundVisitor::IsBound(mixed_and); + EXPECT_THAT(result, HasErrorMessage("Found partially bound expression")); } TEST_F(IsBoundVisitorTest, OrWithBoundChildren) { @@ -354,14 +354,14 @@ TEST_F(IsBoundVisitorTest, OrWithBoundChildren) { } TEST_F(IsBoundVisitorTest, OrWithUnboundChild) { - // OR with any unbound child should return false + // OR with mixed bound/unbound children should error auto pred1 = Expressions::IsNull("name"); // unbound auto bound_pred2 = Expressions::Equal("age", Literal::Int(25)); ICEBERG_UNWRAP_OR_FAIL(auto pred2, Bind(bound_pred2)); auto mixed_or = Expressions::Or(pred1, pred2); - ICEBERG_UNWRAP_OR_FAIL(auto is_bound, IsBoundVisitor::IsBound(mixed_or)); - EXPECT_FALSE(is_bound); + auto result = IsBoundVisitor::IsBound(mixed_or); + EXPECT_THAT(result, HasErrorMessage("Found partially bound expression")); } TEST_F(IsBoundVisitorTest, NotWithBoundChild) { @@ -395,15 +395,15 @@ TEST_F(IsBoundVisitorTest, ComplexExpression) { ICEBERG_UNWRAP_OR_FAIL(auto is_bound, IsBoundVisitor::IsBound(bound_complex)); EXPECT_TRUE(is_bound); - // Complex expression: one unbound should return false + // Complex expression: mixed bound/unbound children should error auto unbound_pred = Expressions::Equal("name", Literal::String("Alice")); ICEBERG_UNWRAP_OR_FAIL(auto bound_pred2, Bind(pred2)); ICEBERG_UNWRAP_OR_FAIL(auto bound_pred3, Bind(pred3)); auto mixed_and = Expressions::And(unbound_pred, bound_pred2); auto mixed_complex = Expressions::Or(mixed_and, bound_pred3); - ICEBERG_UNWRAP_OR_FAIL(auto is_bound_mixed, IsBoundVisitor::IsBound(mixed_complex)); - EXPECT_FALSE(is_bound_mixed); + auto result_mixed = IsBoundVisitor::IsBound(mixed_complex); + EXPECT_THAT(result_mixed, HasErrorMessage("Found partially bound expression")); } class RewriteNotTest : public ExpressionVisitorTest {};