Skip to content

Commit 52c97f9

Browse files
committed
error out when meet unbound expr
1 parent f2674e8 commit 52c97f9

File tree

2 files changed

+12
-13
lines changed

2 files changed

+12
-13
lines changed

src/iceberg/expression/binder.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "iceberg/expression/binder.h"
2121

22+
#include "iceberg/result.h"
2223
#include "iceberg/util/macros.h"
2324

2425
namespace iceberg {
@@ -151,7 +152,7 @@ Result<FieldIdsSetRef> ReferenceVisitor::Predicate(
151152

152153
Result<FieldIdsSetRef> ReferenceVisitor::Predicate(
153154
[[maybe_unused]] const std::shared_ptr<UnboundPredicate>& pred) {
154-
return referenced_field_ids_;
155+
return InvalidExpression("Cannot get referenced field IDs from unbound predicate");
155156
}
156157

157158
Result<FieldIdsSetRef> ReferenceVisitor::Aggregate(
@@ -162,7 +163,7 @@ Result<FieldIdsSetRef> ReferenceVisitor::Aggregate(
162163

163164
Result<FieldIdsSetRef> ReferenceVisitor::Aggregate(
164165
[[maybe_unused]] const std::shared_ptr<UnboundAggregate>& aggregate) {
165-
return referenced_field_ids_;
166+
return InvalidExpression("Cannot get referenced field IDs from unbound aggregate");
166167
}
167168

168169
} // namespace iceberg

src/iceberg/test/expression_visitor_test.cc

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "iceberg/expression/binder.h"
2323
#include "iceberg/expression/expressions.h"
2424
#include "iceberg/expression/rewrite_not.h"
25+
#include "iceberg/result.h"
2526
#include "iceberg/schema.h"
2627
#include "iceberg/test/matchers.h"
2728
#include "iceberg/type.h"
@@ -521,11 +522,11 @@ TEST_F(ReferenceVisitorTest, Constants) {
521522
}
522523

523524
TEST_F(ReferenceVisitorTest, UnboundPredicate) {
524-
// Unbound predicates should have no referenced field IDs (not yet bound to schema)
525525
auto unbound_pred = Expressions::Equal("name", Literal::String("Alice"));
526-
ICEBERG_UNWRAP_OR_FAIL(auto refs,
527-
ReferenceVisitor::GetReferencedFieldIds(unbound_pred));
528-
EXPECT_TRUE(refs.empty());
526+
auto result = ReferenceVisitor::GetReferencedFieldIds(unbound_pred);
527+
EXPECT_THAT(result, IsError(ErrorKind::kInvalidExpression));
528+
EXPECT_THAT(result,
529+
HasErrorMessage("Cannot get referenced field IDs from unbound predicate"));
529530
}
530531

531532
TEST_F(ReferenceVisitorTest, BoundPredicate) {
@@ -672,18 +673,15 @@ TEST_F(ReferenceVisitorTest, SetPredicates) {
672673
}
673674

674675
TEST_F(ReferenceVisitorTest, MixedBoundAndUnbound) {
675-
// Expression with both bound and unbound predicates
676676
auto bound_pred = Expressions::Equal("name", Literal::String("Alice"));
677677
ICEBERG_UNWRAP_OR_FAIL(auto pred1, Bind(bound_pred));
678-
679678
auto unbound_pred = Expressions::GreaterThan("age", Literal::Int(25));
680-
681679
auto mixed_and = Expressions::And(pred1, unbound_pred);
682-
ICEBERG_UNWRAP_OR_FAIL(auto refs, ReferenceVisitor::GetReferencedFieldIds(mixed_and));
683680

684-
// Should only return field IDs from bound predicates
685-
EXPECT_EQ(refs.size(), 1);
686-
EXPECT_EQ(refs.count(2), 1); // name field only
681+
auto result = ReferenceVisitor::GetReferencedFieldIds(mixed_and);
682+
EXPECT_THAT(result, IsError(ErrorKind::kInvalidExpression));
683+
EXPECT_THAT(result,
684+
HasErrorMessage("Cannot get referenced field IDs from unbound predicate"));
687685
}
688686

689687
TEST_F(ReferenceVisitorTest, AllFields) {

0 commit comments

Comments
 (0)