Skip to content

Commit 19b086e

Browse files
wgtmacwgtmac
authored
fix: BoundVisitor should accept Bound instead of BoundTerm (#375)
1 parent df611c2 commit 19b086e

File tree

6 files changed

+132
-131
lines changed

6 files changed

+132
-131
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.h‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include "iceberg/iceberg_export.h"
2929
#include "iceberg/result.h"
30+
#include "iceberg/type_fwd.h"
3031
#include "iceberg/util/formattable.h"
3132
#include "iceberg/util/macros.h"
3233

@@ -328,4 +329,43 @@ ICEBERG_EXPORT std::string_view ToString(Expression::Operation op);
328329
/// \brief Returns the negated operation.
329330
ICEBERG_EXPORT Result<Expression::Operation> Negate(Expression::Operation op);
330331

332+
/// \brief Interface for unbound expressions that need schema binding.
333+
///
334+
/// Unbound expressions contain string-based references that must be resolved
335+
/// against a concrete schema to produce bound expressions that can be evaluated.
336+
///
337+
/// \tparam B The bound type this term produces when binding is successful
338+
template <typename B>
339+
class ICEBERG_EXPORT Unbound {
340+
public:
341+
/// \brief Bind this expression to a concrete schema.
342+
///
343+
/// \param schema The schema to bind against
344+
/// \param case_sensitive Whether field name matching should be case sensitive
345+
/// \return A bound expression or an error if binding fails
346+
virtual Result<std::shared_ptr<B>> Bind(const Schema& schema,
347+
bool case_sensitive) const = 0;
348+
349+
/// \brief Overloaded Bind method that uses case-sensitive matching by default.
350+
Result<std::shared_ptr<B>> Bind(const Schema& schema) const;
351+
352+
/// \brief Returns the underlying named reference for this unbound term.
353+
virtual std::shared_ptr<class NamedReference> reference() = 0;
354+
};
355+
356+
/// \brief Interface for bound expressions that can be evaluated.
357+
///
358+
/// Bound expressions have been resolved against a concrete schema and contain
359+
/// all necessary information to evaluate against data structures.
360+
class ICEBERG_EXPORT Bound {
361+
public:
362+
virtual ~Bound();
363+
364+
/// \brief Evaluate this expression against a row-based data.
365+
virtual Result<Literal> Evaluate(const StructLike& data) const = 0;
366+
367+
/// \brief Returns the underlying bound reference for this term.
368+
virtual std::shared_ptr<class BoundReference> reference() = 0;
369+
};
370+
331371
} // namespace iceberg

‎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/expression/term.h‎

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <string>
2828
#include <string_view>
2929

30+
#include "iceberg/expression/expression.h"
3031
#include "iceberg/expression/literal.h"
3132
#include "iceberg/type_fwd.h"
3233
#include "iceberg/util/formattable.h"
@@ -45,45 +46,6 @@ class ICEBERG_EXPORT Term : public util::Formattable {
4546
template <typename T>
4647
concept TermType = std::derived_from<T, Term>;
4748

48-
/// \brief Interface for unbound expressions that need schema binding.
49-
///
50-
/// Unbound expressions contain string-based references that must be resolved
51-
/// against a concrete schema to produce bound expressions that can be evaluated.
52-
///
53-
/// \tparam B The bound type this term produces when binding is successful
54-
template <typename B>
55-
class ICEBERG_EXPORT Unbound {
56-
public:
57-
/// \brief Bind this expression to a concrete schema.
58-
///
59-
/// \param schema The schema to bind against
60-
/// \param case_sensitive Whether field name matching should be case sensitive
61-
/// \return A bound expression or an error if binding fails
62-
virtual Result<std::shared_ptr<B>> Bind(const Schema& schema,
63-
bool case_sensitive) const = 0;
64-
65-
/// \brief Overloaded Bind method that uses case-sensitive matching by default.
66-
Result<std::shared_ptr<B>> Bind(const Schema& schema) const;
67-
68-
/// \brief Returns the underlying named reference for this unbound term.
69-
virtual std::shared_ptr<class NamedReference> reference() = 0;
70-
};
71-
72-
/// \brief Interface for bound expressions that can be evaluated.
73-
///
74-
/// Bound expressions have been resolved against a concrete schema and contain
75-
/// all necessary information to evaluate against data structures.
76-
class ICEBERG_EXPORT Bound {
77-
public:
78-
virtual ~Bound();
79-
80-
/// \brief Evaluate this expression against a row-based data.
81-
virtual Result<Literal> Evaluate(const StructLike& data) const = 0;
82-
83-
/// \brief Returns the underlying bound reference for this term.
84-
virtual std::shared_ptr<class BoundReference> reference() = 0;
85-
};
86-
8749
/// \brief Base class for unbound terms.
8850
///
8951
/// \tparam B The bound type this term produces when binding is successful.

0 commit comments

Comments
 (0)