Skip to content

Commit f896876

Browse files
wgtmacwgtmac
committed
fix: BoundVisitor should accept Bound instead of BoundTerm
1 parent 3691a5c commit f896876

File tree

4 files changed

+91
-92
lines changed

4 files changed

+91
-92
lines changed

‎src/iceberg/expression/evaluator.cc‎

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -44,72 +44,71 @@ class EvalVisitor : public BoundVisitor<bool> {
4444
return left_result || right_result;
4545
}
4646

47-
Result<bool> IsNull(const std::shared_ptr<BoundTerm>& term) override {
48-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
47+
Result<bool> IsNull(const std::shared_ptr<Bound>& expr) override {
48+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
4949
return value.IsNull();
5050
}
5151

52-
Result<bool> NotNull(const std::shared_ptr<BoundTerm>& term) override {
53-
ICEBERG_ASSIGN_OR_RAISE(auto value, IsNull(term));
52+
Result<bool> NotNull(const std::shared_ptr<Bound>& expr) override {
53+
ICEBERG_ASSIGN_OR_RAISE(auto value, IsNull(expr));
5454
return !value;
5555
}
5656

57-
Result<bool> IsNaN(const std::shared_ptr<BoundTerm>& term) override {
58-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
57+
Result<bool> IsNaN(const std::shared_ptr<Bound>& expr) override {
58+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
5959
return value.IsNaN();
6060
}
6161

62-
Result<bool> NotNaN(const std::shared_ptr<BoundTerm>& term) override {
63-
ICEBERG_ASSIGN_OR_RAISE(auto value, IsNaN(term));
62+
Result<bool> NotNaN(const std::shared_ptr<Bound>& expr) override {
63+
ICEBERG_ASSIGN_OR_RAISE(auto value, IsNaN(expr));
6464
return !value;
6565
}
6666

67-
Result<bool> Lt(const std::shared_ptr<BoundTerm>& term, const Literal& lit) override {
68-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
67+
Result<bool> Lt(const std::shared_ptr<Bound>& expr, const Literal& lit) override {
68+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
6969
return value < lit;
7070
}
7171

72-
Result<bool> LtEq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) override {
73-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
72+
Result<bool> LtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override {
73+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
7474
return value <= lit;
7575
}
7676

77-
Result<bool> Gt(const std::shared_ptr<BoundTerm>& term, const Literal& lit) override {
78-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
77+
Result<bool> Gt(const std::shared_ptr<Bound>& expr, const Literal& lit) override {
78+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
7979
return value > lit;
8080
}
8181

82-
Result<bool> GtEq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) override {
83-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
82+
Result<bool> GtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override {
83+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
8484
return value >= lit;
8585
}
8686

87-
Result<bool> Eq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) override {
88-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
87+
Result<bool> Eq(const std::shared_ptr<Bound>& expr, const Literal& lit) override {
88+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
8989
return value == lit;
9090
}
9191

92-
Result<bool> NotEq(const std::shared_ptr<BoundTerm>& term,
93-
const Literal& lit) override {
94-
ICEBERG_ASSIGN_OR_RAISE(auto eq_result, Eq(term, lit));
92+
Result<bool> NotEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override {
93+
ICEBERG_ASSIGN_OR_RAISE(auto eq_result, Eq(expr, lit));
9594
return !eq_result;
9695
}
9796

98-
Result<bool> In(const std::shared_ptr<BoundTerm>& term,
97+
Result<bool> In(const std::shared_ptr<Bound>& expr,
9998
const BoundSetPredicate::LiteralSet& literal_set) override {
100-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
99+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
101100
return literal_set.contains(value);
102101
}
103102

104-
Result<bool> NotIn(const std::shared_ptr<BoundTerm>& term,
103+
Result<bool> NotIn(const std::shared_ptr<Bound>& expr,
105104
const BoundSetPredicate::LiteralSet& literal_set) override {
106-
ICEBERG_ASSIGN_OR_RAISE(auto in_result, In(term, literal_set));
105+
ICEBERG_ASSIGN_OR_RAISE(auto in_result, In(expr, literal_set));
107106
return !in_result;
108107
}
109108

110-
Result<bool> StartsWith(const std::shared_ptr<BoundTerm>& term,
109+
Result<bool> StartsWith(const std::shared_ptr<Bound>& expr,
111110
const Literal& lit) override {
112-
ICEBERG_ASSIGN_OR_RAISE(auto value, term->Evaluate(row_));
111+
ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(row_));
113112

114113
// Both value and literal should be strings
115114
if (!std::holds_alternative<std::string>(value.value()) ||
@@ -122,9 +121,9 @@ class EvalVisitor : public BoundVisitor<bool> {
122121
return str_value.starts_with(str_prefix);
123122
}
124123

125-
Result<bool> NotStartsWith(const std::shared_ptr<BoundTerm>& term,
124+
Result<bool> NotStartsWith(const std::shared_ptr<Bound>& expr,
126125
const Literal& lit) override {
127-
ICEBERG_ASSIGN_OR_RAISE(auto starts_result, StartsWith(term, lit));
126+
ICEBERG_ASSIGN_OR_RAISE(auto starts_result, StartsWith(expr, lit));
128127
return !starts_result;
129128
}
130129

@@ -144,7 +143,7 @@ Result<std::unique_ptr<Evaluator>> Evaluator::Make(const Schema& schema,
144143
return std::unique_ptr<Evaluator>(new Evaluator(std::move(bound_expr)));
145144
}
146145

147-
Result<bool> Evaluator::Eval(const StructLike& row) const {
146+
Result<bool> Evaluator::Evaluate(const StructLike& row) const {
148147
EvalVisitor visitor(row);
149148
return Visit<bool, EvalVisitor>(bound_expr_, visitor);
150149
}

‎src/iceberg/expression/evaluator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class ICEBERG_EXPORT Evaluator {
5454
///
5555
/// \param row The data row to evaluate
5656
/// \return true if the row matches the expression, false otherwise, or error
57-
Result<bool> Eval(const StructLike& row) const;
57+
Result<bool> Evaluate(const StructLike& row) const;
5858

5959
private:
6060
explicit Evaluator(std::shared_ptr<Expression> bound_expr);

‎src/iceberg/expression/expression_visitor.h‎

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -107,86 +107,86 @@ class ICEBERG_EXPORT BoundVisitor : public ExpressionVisitor<R> {
107107
public:
108108
~BoundVisitor() override = default;
109109

110-
/// \brief Visit an IS_NULL unary predicate.
111-
/// \param term The bound term being tested
112-
virtual Result<R> IsNull(const std::shared_ptr<BoundTerm>& term) = 0;
110+
/// \brief Visit an IS_NULL bound expression.
111+
/// \param expr The bound expression being tested
112+
virtual Result<R> IsNull(const std::shared_ptr<Bound>& expr) = 0;
113113

114-
/// \brief Visit a NOT_NULL unary predicate.
115-
/// \param term The bound term being tested
116-
virtual Result<R> NotNull(const std::shared_ptr<BoundTerm>& term) = 0;
114+
/// \brief Visit a NOT_NULL bound expression.
115+
/// \param expr The bound expression being tested
116+
virtual Result<R> NotNull(const std::shared_ptr<Bound>& expr) = 0;
117117

118-
/// \brief Visit an IS_NAN unary predicate.
119-
/// \param term The bound term being tested
120-
virtual Result<R> IsNaN(const std::shared_ptr<BoundTerm>& term) {
118+
/// \brief Visit an IS_NAN bound expression.
119+
/// \param expr The bound expression being tested
120+
virtual Result<R> IsNaN(const std::shared_ptr<Bound>& expr) {
121121
return NotSupported("IsNaN operation is not supported by this visitor");
122122
}
123123

124-
/// \brief Visit a NOT_NAN unary predicate.
125-
/// \param term The bound term being tested
126-
virtual Result<R> NotNaN(const std::shared_ptr<BoundTerm>& term) {
124+
/// \brief Visit a NOT_NAN bound expression.
125+
/// \param expr The bound expression being tested
126+
virtual Result<R> NotNaN(const std::shared_ptr<Bound>& expr) {
127127
return NotSupported("NotNaN operation is not supported by this visitor");
128128
}
129129

130-
/// \brief Visit a less-than predicate.
131-
/// \param term The bound term
130+
/// \brief Visit a less-than bound expression.
131+
/// \param expr The bound expression being tested
132132
/// \param lit The literal value to compare against
133-
virtual Result<R> Lt(const std::shared_ptr<BoundTerm>& term, const Literal& lit) = 0;
133+
virtual Result<R> Lt(const std::shared_ptr<Bound>& expr, const Literal& lit) = 0;
134134

135-
/// \brief Visit a less-than-or-equal predicate.
136-
/// \param term The bound term
135+
/// \brief Visit a less-than-or-equal bound expression.
136+
/// \param expr The bound expression being tested
137137
/// \param lit The literal value to compare against
138-
virtual Result<R> LtEq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) = 0;
138+
virtual Result<R> LtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) = 0;
139139

140-
/// \brief Visit a greater-than predicate.
141-
/// \param term The bound term
140+
/// \brief Visit a greater-than bound expression.
141+
/// \param expr The bound expression being tested
142142
/// \param lit The literal value to compare against
143-
virtual Result<R> Gt(const std::shared_ptr<BoundTerm>& term, const Literal& lit) = 0;
143+
virtual Result<R> Gt(const std::shared_ptr<Bound>& expr, const Literal& lit) = 0;
144144

145-
/// \brief Visit a greater-than-or-equal predicate.
146-
/// \param term The bound term
145+
/// \brief Visit a greater-than-or-equal bound expression.
146+
/// \param expr The bound expression being tested
147147
/// \param lit The literal value to compare against
148-
virtual Result<R> GtEq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) = 0;
148+
virtual Result<R> GtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) = 0;
149149

150-
/// \brief Visit an equality predicate.
151-
/// \param term The bound term
150+
/// \brief Visit an equality bound expression.
151+
/// \param expr The bound expression being tested
152152
/// \param lit The literal value to compare against
153-
virtual Result<R> Eq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) = 0;
153+
virtual Result<R> Eq(const std::shared_ptr<Bound>& expr, const Literal& lit) = 0;
154154

155-
/// \brief Visit a not-equal predicate.
156-
/// \param term The bound term
155+
/// \brief Visit a not-equal bound expression.
156+
/// \param expr The bound expression being tested
157157
/// \param lit The literal value to compare against
158-
virtual Result<R> NotEq(const std::shared_ptr<BoundTerm>& term, const Literal& lit) = 0;
158+
virtual Result<R> NotEq(const std::shared_ptr<Bound>& expr, const Literal& lit) = 0;
159159

160-
/// \brief Visit a starts-with predicate.
161-
/// \param term The bound term
160+
/// \brief Visit a starts-with bound expression.
161+
/// \param expr The bound expression being tested
162162
/// \param lit The literal value to check for prefix match
163-
virtual Result<R> StartsWith([[maybe_unused]] const std::shared_ptr<BoundTerm>& term,
163+
virtual Result<R> StartsWith([[maybe_unused]] const std::shared_ptr<Bound>& expr,
164164
[[maybe_unused]] const Literal& lit) {
165165
return NotSupported("StartsWith operation is not supported by this visitor");
166166
}
167167

168-
/// \brief Visit a not-starts-with predicate.
169-
/// \param term The bound term
168+
/// \brief Visit a not-starts-with bound expression.
169+
/// \param expr The bound expression being tested
170170
/// \param lit The literal value to check for prefix match
171-
virtual Result<R> NotStartsWith([[maybe_unused]] const std::shared_ptr<BoundTerm>& term,
171+
virtual Result<R> NotStartsWith([[maybe_unused]] const std::shared_ptr<Bound>& expr,
172172
[[maybe_unused]] const Literal& lit) {
173173
return NotSupported("NotStartsWith operation is not supported by this visitor");
174174
}
175175

176-
/// \brief Visit an IN set predicate.
177-
/// \param term The bound term
176+
/// \brief Visit an IN set bound expression.
177+
/// \param expr The bound expression being tested
178178
/// \param literal_set The set of literal values to test membership
179179
virtual Result<R> In(
180-
[[maybe_unused]] const std::shared_ptr<BoundTerm>& term,
180+
[[maybe_unused]] const std::shared_ptr<Bound>& expr,
181181
[[maybe_unused]] const BoundSetPredicate::LiteralSet& literal_set) {
182182
return NotSupported("In operation is not supported by this visitor");
183183
}
184184

185-
/// \brief Visit a NOT_IN set predicate.
186-
/// \param term The bound term
185+
/// \brief Visit a NOT_IN set bound expression.
186+
/// \param expr The bound expression being tested
187187
/// \param literal_set The set of literal values to test membership
188188
virtual Result<R> NotIn(
189-
[[maybe_unused]] const std::shared_ptr<BoundTerm>& term,
189+
[[maybe_unused]] const std::shared_ptr<Bound>& expr,
190190
[[maybe_unused]] const BoundSetPredicate::LiteralSet& literal_set) {
191191
return NotSupported("NotIn operation is not supported by this visitor");
192192
}

‎src/iceberg/test/evaluator_test.cc‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ class EvaluatorTest : public ::testing::Test {
121121

122122
ICEBERG_UNWRAP_OR_FAIL(auto struct_like,
123123
ArrowArrayStructLike::Make(arrow_c_schema_, arrow_c_array, 0));
124-
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator.Eval(*struct_like));
124+
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator.Evaluate(*struct_like));
125125
ASSERT_EQ(result, expected_result);
126126
}
127127

@@ -357,32 +357,32 @@ TEST_F(EvaluatorTest, StartsWith) {
357357

358358
// abc startsWith abc => true
359359
ASSERT_THAT(struct_like->Reset(0), IsOk());
360-
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Eval(*struct_like));
360+
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Evaluate(*struct_like));
361361
EXPECT_TRUE(result);
362362

363363
// xabc startsWith abc => false
364364
ASSERT_THAT(struct_like->Reset(1), IsOk());
365-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
365+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
366366
EXPECT_FALSE(result);
367367

368368
// Abc startsWith abc => false
369369
ASSERT_THAT(struct_like->Reset(2), IsOk());
370-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
370+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
371371
EXPECT_FALSE(result);
372372

373373
// a startsWith abc => false
374374
ASSERT_THAT(struct_like->Reset(3), IsOk());
375-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
375+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
376376
EXPECT_FALSE(result);
377377

378378
// abcd startsWith abc => true
379379
ASSERT_THAT(struct_like->Reset(4), IsOk());
380-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
380+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
381381
EXPECT_TRUE(result);
382382

383383
// null startsWith abc => false
384384
ASSERT_THAT(struct_like->Reset(5), IsOk());
385-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
385+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
386386
EXPECT_FALSE(result);
387387
}
388388

@@ -416,32 +416,32 @@ TEST_F(EvaluatorTest, NotStartsWith) {
416416

417417
// abc notStartsWith abc => false
418418
ASSERT_THAT(struct_like->Reset(0), IsOk());
419-
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Eval(*struct_like));
419+
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Evaluate(*struct_like));
420420
EXPECT_FALSE(result);
421421

422422
// xabc notStartsWith abc => true
423423
ASSERT_THAT(struct_like->Reset(1), IsOk());
424-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
424+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
425425
EXPECT_TRUE(result);
426426

427427
// Abc notStartsWith abc => true
428428
ASSERT_THAT(struct_like->Reset(2), IsOk());
429-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
429+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
430430
EXPECT_TRUE(result);
431431

432432
// a notStartsWith abc => true
433433
ASSERT_THAT(struct_like->Reset(3), IsOk());
434-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
434+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
435435
EXPECT_TRUE(result);
436436

437437
// abcde notStartsWith abc => false
438438
ASSERT_THAT(struct_like->Reset(4), IsOk());
439-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
439+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
440440
EXPECT_FALSE(result);
441441

442442
// Abcde notStartsWith abc => true
443443
ASSERT_THAT(struct_like->Reset(5), IsOk());
444-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
444+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
445445
EXPECT_TRUE(result);
446446
}
447447

@@ -533,17 +533,17 @@ TEST_F(EvaluatorTest, IsNaN) {
533533

534534
// NaN is NaN => true
535535
ASSERT_THAT(struct_like->Reset(0), IsOk());
536-
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Eval(*struct_like));
536+
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Evaluate(*struct_like));
537537
EXPECT_TRUE(result);
538538

539539
// 2.0 is not NaN => false
540540
ASSERT_THAT(struct_like->Reset(1), IsOk());
541-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
541+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
542542
EXPECT_FALSE(result);
543543

544544
// Infinity is not NaN => false
545545
ASSERT_THAT(struct_like->Reset(2), IsOk());
546-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
546+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
547547
EXPECT_FALSE(result);
548548
}
549549

@@ -578,17 +578,17 @@ TEST_F(EvaluatorTest, NotNaN) {
578578

579579
// NaN is NaN => false
580580
ASSERT_THAT(struct_like->Reset(0), IsOk());
581-
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Eval(*struct_like));
581+
ICEBERG_UNWRAP_OR_FAIL(auto result, evaluator->Evaluate(*struct_like));
582582
EXPECT_FALSE(result);
583583

584584
// 2.0 is not NaN => true
585585
ASSERT_THAT(struct_like->Reset(1), IsOk());
586-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
586+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
587587
EXPECT_TRUE(result);
588588

589589
// Infinity is not NaN => true
590590
ASSERT_THAT(struct_like->Reset(2), IsOk());
591-
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Eval(*struct_like));
591+
ICEBERG_UNWRAP_OR_FAIL(result, evaluator->Evaluate(*struct_like));
592592
EXPECT_TRUE(result);
593593
}
594594

0 commit comments

Comments
 (0)