YinZheng-Sun commented on code in PR #956:
URL: https://github.com/apache/iceberg-cpp/pull/956#discussion_r4237257157
##########
src/iceberg/expression/binder.h:
##########
@@ -61,19 +62,25 @@ class ICEBERG_EXPORT Binder : public
ExpressionVisitor<std::shared_ptr<Expressio
const bool case_sensitive_;
};
-class ICEBERG_EXPORT IsBoundVisitor : public ExpressionVisitor<bool> {
+class ICEBERG_EXPORT IsBoundVisitor : public
ExpressionVisitor<std::optional<bool>> {
Review Comment:
The optional represents an internal traversal state, not uncertainty about
whether an expression is bound. nullopt means that the subtree contains no
references, as with True or False.
Constants need to be neutral when combining binding states: And(True,
unbound) should be unbound, while And(True, bound) should be bound. Mixed bound
and unbound predicates are still rejected. The public IsBound() API continues
to return a definite boolean.
This follows Java’s IsBoundVisitor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
