nullccxsy commented on code in PR #206:
URL: https://github.com/apache/iceberg-cpp/pull/206#discussion_r2320665309
##########
src/iceberg/expression/literal.cc:
##########
@@ -26,6 +26,16 @@
namespace iceberg {
+namespace {
+
+constexpr int64_t kMicrosPerDay = 86400000000LL; // 24 * 60 * 60 * 1000 * 1000
+
+int32_t MicrosToDays(int64_t micros) {
+ return static_cast<int32_t>(std::floor(static_cast<double>(micros) /
kMicrosPerDay));
Review Comment:
we can use `static_cast<int32_t>(micros / kMicrosPerDay))`
##########
src/iceberg/expression/literal.cc:
##########
@@ -120,6 +169,131 @@ Result<Literal> LiteralCaster::CastFromFloat(
}
}
+Result<Literal> LiteralCaster::CastFromDouble(
+ const Literal& literal, const std::shared_ptr<PrimitiveType>& target_type)
{
+ auto double_val = std::get<double>(literal.value_);
+
+ switch (target_type->type_id()) {
+ case TypeId::kFloat: {
+ if (double_val > static_cast<double>(std::numeric_limits<float>::max()))
{
+ return AboveMaxLiteral(target_type);
+ }
+ if (double_val <
-static_cast<double>(std::numeric_limits<float>::max())) {
Review Comment:
`-static_cast<double>(std::numeric_limits<float>::max())` can change to
`static_cast<double>(std::numeric_limits<float>::lowest())`
##########
src/iceberg/expression/literal.cc:
##########
@@ -285,22 +466,28 @@ std::string Literal::ToString() const {
case TypeId::kString: {
return std::get<std::string>(value_);
}
- case TypeId::kBinary: {
+ case TypeId::kBinary:
+ case TypeId::kFixed: {
const auto& binary_data = std::get<std::vector<uint8_t>>(value_);
- std::string result;
- result.reserve(binary_data.size() * 2); // 2 chars per byte
+ std::string result = "X'";
+ result.reserve(2 + binary_data.size() * 2 +
+ 1); // 2 chars per byte and 2 + 1 for prefix and suffix
for (const auto& byte : binary_data) {
std::format_to(std::back_inserter(result), "{:02X}", byte);
}
+ result.push_back('\'');
return result;
}
- case TypeId::kDecimal:
- case TypeId::kUuid:
- case TypeId::kFixed:
- case TypeId::kDate:
case TypeId::kTime:
case TypeId::kTimestamp:
case TypeId::kTimestampTz: {
+ return std::to_string(std::get<int64_t>(value_));
+ }
+ case TypeId::kDate: {
+ return std::to_string(std::get<int32_t>(value_));
+ }
+ case TypeId::kDecimal:
+ case TypeId::kUuid: {
throw IcebergError("Not implemented: ToString for " + type_->ToString());
Review Comment:
There we should `return NotImplement(....)`
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
