Skip to content

Commit c63614e

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 06ce20f commit c63614e

18 files changed

+266
-193
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
@@ -26,6 +26,7 @@
2626
#include <type_traits>
2727
#include <unordered_set>
2828

29+
#include <iceberg/util/timepoint.h>
2930
#include <nlohmann/json.hpp>
3031

3132
#include "iceberg/partition_spec.h"
@@ -499,7 +500,7 @@ nlohmann::json ToJson(const Snapshot& snapshot) {
499500
if (snapshot.sequence_number > TableMetadata::kInitialSequenceNumber) {
500501
json[kSequenceNumber] = snapshot.sequence_number;
501502
}
502-
json[kTimestampMs] = snapshot.timestamp_ms;
503+
json[kTimestampMs] = UnixMsFromTimePointMs(snapshot.timestamp_ms);
503504
json[kManifestList] = snapshot.manifest_list;
504505
// If there is an operation, write the summary map
505506
if (snapshot.operation().has_value()) {
@@ -712,7 +713,9 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
712713
ICEBERG_ASSIGN_OR_RAISE(auto snapshot_id, GetJsonValue<int64_t>(json, kSnapshotId));
713714
ICEBERG_ASSIGN_OR_RAISE(auto sequence_number,
714715
GetJsonValueOptional<int64_t>(json, kSequenceNumber));
715-
ICEBERG_ASSIGN_OR_RAISE(auto timestamp_ms, GetJsonValue<int64_t>(json, kTimestampMs));
716+
ICEBERG_ASSIGN_OR_RAISE(
717+
auto timestamp_ms,
718+
GetJsonValue<int64_t>(json, kTimestampMs).and_then(TimePointMsFromUnixMs));
716719
ICEBERG_ASSIGN_OR_RAISE(auto manifest_list,
717720
GetJsonValue<std::string>(json, kManifestList));
718721

@@ -725,24 +728,14 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
725728
if (summary_json.has_value()) {
726729
for (const auto& [key, value] : summary_json->items()) {
727730
if (!kValidSnapshotSummaryFields.contains(key)) {
728-
return unexpected<Error>({
729-
.kind = ErrorKind::kJsonParseError,
730-
.message = std::format("Invalid snapshot summary field: {}", key),
731-
});
731+
return JsonParseError("Invalid snapshot summary field: {}", key);
732732
}
733733
if (!value.is_string()) {
734-
return unexpected<Error>({
735-
.kind = ErrorKind::kJsonParseError,
736-
.message =
737-
std::format("Invalid snapshot summary field value: {}", value.dump()),
738-
});
734+
return JsonParseError("Invalid snapshot summary field value: {}", value.dump());
739735
}
740736
if (key == SnapshotSummaryFields::kOperation &&
741737
!kValidDataOperation.contains(value.get<std::string>())) {
742-
return unexpected<Error>({
743-
.kind = ErrorKind::kJsonParseError,
744-
.message = std::format("Invalid snapshot operation: {}", value.dump()),
745-
});
738+
return JsonParseError("Invalid snapshot operation: {}", value.dump());
746739
}
747740
summary[key] = value.get<std::string>();
748741
}

‎src/iceberg/result.h‎

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,19 @@
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,
35+
kInvalidExpression,
36+
kInvalidSchema,
3937
kIOError,
38+
kJsonParseError,
39+
kNoSuchNamespace,
40+
kNoSuchTable,
4041
kNotImplemented,
41-
kUnknownError,
4242
kNotSupported,
43-
kInvalidExpression,
44-
kJsonParseError,
43+
kUnknownError,
4544
};
4645

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

6362
using Status = Result<void>;
6463

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

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

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

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

0 commit comments

Comments
 (0)