Skip to content

Commit ef0e808

Browse files
zhjwpkuzhjwpku
committed
chore: use chrono::milliseconds in snapshot and consolidate error usage
reduce direct unexpected usage by using more Error wrappers defined in result.h Signed-off-by: Junwang Zhao <[email protected]>
1 parent 2aed97b commit ef0e808

18 files changed

+267
-194
lines changed

‎src/iceberg/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ set(ICEBERG_SOURCES
3636
transform_function.cc
3737
type.cc
3838
snapshot.cc
39-
util/murmurhash3_internal.cc)
39+
util/murmurhash3_internal.cc
40+
util/timepoint.cc)
4041

4142
set(ICEBERG_STATIC_BUILD_INTERFACE_LIBS)
4243
set(ICEBERG_SHARED_BUILD_INTERFACE_LIBS)

‎src/iceberg/expression/expression.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ class ICEBERG_EXPORT Expression {
6868

6969
/// \brief Returns the negation of this expression, equivalent to not(this).
7070
virtual Result<std::shared_ptr<Expression>> Negate() const {
71-
return unexpected(
72-
Error(ErrorKind::kInvalidExpression, "Expression cannot be negated"));
71+
return InvalidExpressionError("Expression cannot be negated");
7372
}
7473

7574
/// \brief Returns whether this expression will accept the same values as another.

‎src/iceberg/file_io.h‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ class ICEBERG_EXPORT FileIO {
5252
virtual Result<std::string> ReadFile(const std::string& file_location,
5353
std::optional<size_t> length) {
5454
// We provide a default implementation to avoid Windows linker error LNK2019.
55-
return unexpected<Error>{
56-
{.kind = ErrorKind::kNotImplemented, .message = "ReadFile not implemented"}};
55+
return NotImplementedError("ReadFile not implemented");
5756
}
5857

5958
/// \brief Write the given content to the file at the given location.
@@ -64,17 +63,15 @@ class ICEBERG_EXPORT FileIO {
6463
/// file exists.
6564
/// \return void if the write succeeded, an error code if the write failed.
6665
virtual Status WriteFile(const std::string& file_location, std::string_view content) {
67-
return unexpected<Error>{
68-
{.kind = ErrorKind::kNotImplemented, .message = "WriteFile not implemented"}};
66+
return NotImplementedError("WriteFile not implemented");
6967
}
7068

7169
/// \brief Delete a file at the given location.
7270
///
7371
/// \param file_location The location of the file to delete.
7472
/// \return void if the delete succeeded, an error code if the delete failed.
7573
virtual Status DeleteFile(const std::string& file_location) {
76-
return unexpected<Error>{
77-
{.kind = ErrorKind::kNotImplemented, .message = "DeleteFile not implemented"}};
74+
return NotImplementedError("DeleteFile not implemented");
7875
}
7976
};
8077

‎src/iceberg/json_internal.cc‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <unordered_set>
2929

3030
#include <iceberg/table.h>
31+
#include <iceberg/util/timepoint.h>
3132
#include <nlohmann/json.hpp>
3233

3334
#include "iceberg/partition_field.h"
@@ -502,7 +503,7 @@ nlohmann::json ToJson(const Snapshot& snapshot) {
502503
if (snapshot.sequence_number > TableMetadata::kInitialSequenceNumber) {
503504
json[kSequenceNumber] = snapshot.sequence_number;
504505
}
505-
json[kTimestampMs] = snapshot.timestamp_ms;
506+
json[kTimestampMs] = UnixMsFromTimePointMs(snapshot.timestamp_ms);
506507
json[kManifestList] = snapshot.manifest_list;
507508
// If there is an operation, write the summary map
508509
if (snapshot.operation().has_value()) {
@@ -722,7 +723,9 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
722723
ICEBERG_ASSIGN_OR_RAISE(auto snapshot_id, GetJsonValue<int64_t>(json, kSnapshotId));
723724
ICEBERG_ASSIGN_OR_RAISE(auto sequence_number,
724725
GetJsonValueOptional<int64_t>(json, kSequenceNumber));
725-
ICEBERG_ASSIGN_OR_RAISE(auto timestamp_ms, GetJsonValue<int64_t>(json, kTimestampMs));
726+
ICEBERG_ASSIGN_OR_RAISE(
727+
auto timestamp_ms,
728+
GetJsonValue<int64_t>(json, kTimestampMs).and_then(TimePointMsFromUnixMs));
726729
ICEBERG_ASSIGN_OR_RAISE(auto manifest_list,
727730
GetJsonValue<std::string>(json, kManifestList));
728731

@@ -735,24 +738,14 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
735738
if (summary_json.has_value()) {
736739
for (const auto& [key, value] : summary_json->items()) {
737740
if (!kValidSnapshotSummaryFields.contains(key)) {
738-
return unexpected<Error>({
739-
.kind = ErrorKind::kJsonParseError,
740-
.message = std::format("Invalid snapshot summary field: {}", key),
741-
});
741+
return JsonParseError("Invalid snapshot summary field: {}", key);
742742
}
743743
if (!value.is_string()) {
744-
return unexpected<Error>({
745-
.kind = ErrorKind::kJsonParseError,
746-
.message =
747-
std::format("Invalid snapshot summary field value: {}", value.dump()),
748-
});
744+
return JsonParseError("Invalid snapshot summary field value: {}", value.dump());
749745
}
750746
if (key == SnapshotSummaryFields::kOperation &&
751747
!kValidDataOperation.contains(value.get<std::string>())) {
752-
return unexpected<Error>({
753-
.kind = ErrorKind::kJsonParseError,
754-
.message = std::format("Invalid snapshot operation: {}", value.dump()),
755-
});
748+
return JsonParseError("Invalid snapshot operation: {}", value.dump());
756749
}
757750
summary[key] = value.get<std::string>();
758751
}

‎src/iceberg/result.h‎

Lines changed: 84 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,20 @@
2828
namespace iceberg {
2929

3030
/// \brief Error types for iceberg.
31-
/// TODO: add more and sort them based on some rules.
3231
enum class ErrorKind {
33-
kNoSuchNamespace,
3432
kAlreadyExists,
35-
kNoSuchTable,
3633
kCommitStateUnknown,
37-
kInvalidSchema,
3834
kInvalidArgument,
39-
kIOError,
40-
kNotImplemented,
41-
kUnknownError,
42-
kNotSupported,
4335
kInvalidExpression,
36+
kInvalidSchema,
37+
kIOError,
4438
kJsonParseError,
39+
kNoSuchNamespace,
40+
kNoSuchTable,
4541
kNotFound,
42+
kNotImplemented,
43+
kNotSupported,
44+
kUnknownError,
4645
};
4746

4847
/// \brief Error with a kind and a message.
@@ -63,19 +62,27 @@ using Result = expected<T, E>;
6362

6463
using Status = Result<void>;
6564

66-
/// \brief Create an unexpected error with kNotImplemented
65+
/// \brief Create an unexpected error with kAlreadyExists
6766
template <typename... Args>
68-
auto NotImplementedError(const std::format_string<Args...> fmt, Args&&... args)
67+
auto AlreadyExistsError(const std::format_string<Args...> fmt, Args&&... args)
6968
-> unexpected<Error> {
70-
return unexpected<Error>({.kind = ErrorKind::kNotImplemented,
69+
return unexpected<Error>({.kind = ErrorKind::kAlreadyExists,
7170
.message = std::format(fmt, std::forward<Args>(args)...)});
7271
}
7372

74-
/// \brief Create an unexpected error with kJsonParseError
73+
/// \brief Create an unexpected error with kCommitStateUnknown
7574
template <typename... Args>
76-
auto JsonParseError(const std::format_string<Args...> fmt, Args&&... args)
75+
auto CommitStateUnknownError(const std::format_string<Args...> fmt, Args&&... args)
7776
-> unexpected<Error> {
78-
return unexpected<Error>({.kind = ErrorKind::kJsonParseError,
77+
return unexpected<Error>({.kind = ErrorKind::kCommitStateUnknown,
78+
.message = std::format(fmt, std::forward<Args>(args)...)});
79+
}
80+
81+
/// \brief Create an unexpected error with kInvalidArgument
82+
template <typename... Args>
83+
auto InvalidArgumentError(const std::format_string<Args...> fmt, Args&&... args)
84+
-> unexpected<Error> {
85+
return unexpected<Error>({.kind = ErrorKind::kInvalidArgument,
7986
.message = std::format(fmt, std::forward<Args>(args)...)});
8087
}
8188

@@ -87,4 +94,67 @@ auto InvalidExpressionError(const std::format_string<Args...> fmt, Args&&... arg
8794
.message = std::format(fmt, std::forward<Args>(args)...)});
8895
}
8996

97+
/// \brief Create an unexpected error with kInvalidSchema
98+
template <typename... Args>
99+
auto InvalidSchemaError(const std::format_string<Args...> fmt, Args&&... args)
100+
-> unexpected<Error> {
101+
return unexpected<Error>({.kind = ErrorKind::kInvalidSchema,
102+
.message = std::format(fmt, std::forward<Args>(args)...)});
103+
}
104+
105+
/// \brief Create an unexpected error with kIOError
106+
template <typename... Args>
107+
auto IOError(const std::format_string<Args...> fmt, Args&&... args) -> unexpected<Error> {
108+
return unexpected<Error>({.kind = ErrorKind::kIOError,
109+
.message = std::format(fmt, std::forward<Args>(args)...)});
110+
}
111+
112+
/// \brief Create an unexpected error with kJsonParseError
113+
template <typename... Args>
114+
auto JsonParseError(const std::format_string<Args...> fmt, Args&&... args)
115+
-> unexpected<Error> {
116+
return unexpected<Error>({.kind = ErrorKind::kJsonParseError,
117+
.message = std::format(fmt, std::forward<Args>(args)...)});
118+
}
119+
120+
/// \brief Create an unexpected error with kNoSuchNamespace
121+
template <typename... Args>
122+
auto NoSuchNamespaceError(const std::format_string<Args...> fmt, Args&&... args)
123+
-> unexpected<Error> {
124+
return unexpected<Error>({.kind = ErrorKind::kNoSuchNamespace,
125+
.message = std::format(fmt, std::forward<Args>(args)...)});
126+
}
127+
128+
/// \brief Create an unexpected error with kNoSuchTable
129+
template <typename... Args>
130+
auto NoSuchTableError(const std::format_string<Args...> fmt, Args&&... args)
131+
-> unexpected<Error> {
132+
return unexpected<Error>({.kind = ErrorKind::kNoSuchTable,
133+
.message = std::format(fmt, std::forward<Args>(args)...)});
134+
}
135+
136+
/// \brief Create an unexpected error with kNotImplemented
137+
template <typename... Args>
138+
auto NotImplementedError(const std::format_string<Args...> fmt, Args&&... args)
139+
-> unexpected<Error> {
140+
return unexpected<Error>({.kind = ErrorKind::kNotImplemented,
141+
.message = std::format(fmt, std::forward<Args>(args)...)});
142+
}
143+
144+
/// \brief Create an unexpected error with kNotSupported
145+
template <typename... Args>
146+
auto NotSupportedError(const std::format_string<Args...> fmt, Args&&... args)
147+
-> unexpected<Error> {
148+
return unexpected<Error>({.kind = ErrorKind::kNotSupported,
149+
.message = std::format(fmt, std::forward<Args>(args)...)});
150+
}
151+
152+
/// \brief Create an unexpected error with kUnknownError
153+
template <typename... Args>
154+
auto UnknownError(const std::format_string<Args...> fmt, Args&&... args)
155+
-> unexpected<Error> {
156+
return unexpected<Error>({.kind = ErrorKind::kUnknownError,
157+
.message = std::format(fmt, std::forward<Args>(args)...)});
158+
}
159+
90160
} // namespace iceberg

0 commit comments

Comments
 (0)