-
Notifications
You must be signed in to change notification settings - Fork 76
feat: add residual evaluator #402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
wgtmac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! I believe the current implementation can be greatly polished to make it not lengthy than the Java impl.
| ICEBERG_ASSIGN_OR_RAISE(auto value, expr->Evaluate(partition_data_)); | ||
| if (value.IsNull()) { | ||
| return Expressions::AlwaysTrue(); | ||
| } | ||
| return Expressions::AlwaysFalse(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make all these functions are a one-liner if we do:
namespace {
std::shared_ptr<Expression> always_true() { return True::Instance(); }
std::shared_ptr<Expression> always_false() { return False::Instance(); }
} // namespace
Result<std::shared_ptr<Expression>> IsNull(
const std::shared_ptr<Bound>& expr) override {
return expr->Evaluate(partition_data_).transform([](const auto& value) {
return value.IsNull() ? always_true() : always_false();
});
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
fc25e14 to
4bfa2b0
Compare
6c8d0fd to
ca7d59f
Compare
No description provided.