wgtmac commented on code in PR #505:
URL: https://github.com/apache/iceberg-cpp/pull/505#discussion_r2685324800
##########
src/iceberg/transform.cc:
##########
@@ -366,6 +367,50 @@ Result<std::unique_ptr<UnboundPredicate>>
Transform::ProjectStrict(
std::unreachable();
}
+Result<std::string> Transform::ToHumanString(const Literal& value) {
+ if (value.IsNull()) {
+ return "null";
+ }
+
+ switch (transform_type_) {
+ case TransformType::kYear:
+ return TransformUtil::HumanYear(std::get<int32_t>(value.value()));
+ case TransformType::kMonth:
+ return TransformUtil::HumanMonth(std::get<int32_t>(value.value()));
+ case TransformType::kDay:
+ return TransformUtil::HumanDay(std::get<int32_t>(value.value()));
+ case TransformType::kHour:
+ return TransformUtil::HumanHour(std::get<int32_t>(value.value()));
+ default: {
Review Comment:
Can we remove this `default` and add all other branches explicitly to avoid
forgetting to update here in the future once we add a new transform type?
##########
src/iceberg/transform.h:
##########
@@ -194,6 +194,12 @@ class ICEBERG_EXPORT Transform : public util::Formattable {
Result<std::unique_ptr<UnboundPredicate>> ProjectStrict(
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate);
+ /// \brief Returns a human-readable String representation of a transformed
value.
+ ///
+ /// \param value The literal value to be transformed.
+ /// \return A human-readable String representation of the value
Review Comment:
```suggestion
/// \return A human-readable string representation of the value
```
##########
src/iceberg/transform.cc:
##########
@@ -366,6 +367,50 @@ Result<std::unique_ptr<UnboundPredicate>>
Transform::ProjectStrict(
std::unreachable();
}
+Result<std::string> Transform::ToHumanString(const Literal& value) {
+ if (value.IsNull()) {
Review Comment:
Let's return error when `IsAboveMax()` or `IsBelowMin()` is true.
##########
src/iceberg/transform.cc:
##########
@@ -366,6 +367,50 @@ Result<std::unique_ptr<UnboundPredicate>>
Transform::ProjectStrict(
std::unreachable();
}
+Result<std::string> Transform::ToHumanString(const Literal& value) {
+ if (value.IsNull()) {
+ return "null";
+ }
+
+ switch (transform_type_) {
+ case TransformType::kYear:
+ return TransformUtil::HumanYear(std::get<int32_t>(value.value()));
Review Comment:
We might need to check the literal type before calling `std::get` on the
variant value to avoid exception.
##########
src/iceberg/transform.h:
##########
@@ -194,6 +194,12 @@ class ICEBERG_EXPORT Transform : public util::Formattable {
Result<std::unique_ptr<UnboundPredicate>> ProjectStrict(
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate);
+ /// \brief Returns a human-readable String representation of a transformed
value.
Review Comment:
```suggestion
/// \brief Returns a human-readable string representation of a transformed
value.
```
--
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]
