Skip to content

Commit f869003

Browse files
zhjwpkuzhjwpku
authored
chore: change return type of TimePointMsFromUnix* functions (#507)
1 parent f842411 commit f869003

File tree

8 files changed

+29
-34
lines changed

8 files changed

+29
-34
lines changed

‎src/iceberg/json_internal.cc‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -645,9 +645,8 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
645645
ICEBERG_ASSIGN_OR_RAISE(auto snapshot_id, GetJsonValue<int64_t>(json, kSnapshotId));
646646
ICEBERG_ASSIGN_OR_RAISE(auto sequence_number,
647647
GetJsonValueOptional<int64_t>(json, kSequenceNumber));
648-
ICEBERG_ASSIGN_OR_RAISE(
649-
auto timestamp_ms,
650-
GetJsonValue<int64_t>(json, kTimestampMs).and_then(TimePointMsFromUnixMs));
648+
ICEBERG_ASSIGN_OR_RAISE(auto unix_ms, GetJsonValue<int64_t>(json, kTimestampMs));
649+
auto timestamp_ms = TimePointMsFromUnixMs(unix_ms);
651650
ICEBERG_ASSIGN_OR_RAISE(auto manifest_list,
652651
GetJsonValue<std::string>(json, kManifestList));
653652

@@ -781,9 +780,8 @@ nlohmann::json ToJson(const SnapshotLogEntry& snapshot_log_entry) {
781780

782781
Result<SnapshotLogEntry> SnapshotLogEntryFromJson(const nlohmann::json& json) {
783782
SnapshotLogEntry snapshot_log_entry;
784-
ICEBERG_ASSIGN_OR_RAISE(
785-
snapshot_log_entry.timestamp_ms,
786-
GetJsonValue<int64_t>(json, kTimestampMs).and_then(TimePointMsFromUnixMs));
783+
ICEBERG_ASSIGN_OR_RAISE(auto unix_ms, GetJsonValue<int64_t>(json, kTimestampMs));
784+
snapshot_log_entry.timestamp_ms = TimePointMsFromUnixMs(unix_ms);
787785
ICEBERG_ASSIGN_OR_RAISE(snapshot_log_entry.snapshot_id,
788786
GetJsonValue<int64_t>(json, kSnapshotId));
789787
return snapshot_log_entry;
@@ -798,9 +796,8 @@ nlohmann::json ToJson(const MetadataLogEntry& metadata_log_entry) {
798796

799797
Result<MetadataLogEntry> MetadataLogEntryFromJson(const nlohmann::json& json) {
800798
MetadataLogEntry metadata_log_entry;
801-
ICEBERG_ASSIGN_OR_RAISE(
802-
metadata_log_entry.timestamp_ms,
803-
GetJsonValue<int64_t>(json, kTimestampMs).and_then(TimePointMsFromUnixMs));
799+
ICEBERG_ASSIGN_OR_RAISE(auto unix_ms, GetJsonValue<int64_t>(json, kTimestampMs));
800+
metadata_log_entry.timestamp_ms = TimePointMsFromUnixMs(unix_ms);
804801
ICEBERG_ASSIGN_OR_RAISE(metadata_log_entry.metadata_file,
805802
GetJsonValue<std::string>(json, kMetadataFile));
806803
return metadata_log_entry;

‎src/iceberg/table_scan.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,7 @@ TableScanBuilder& TableScanBuilder::UseRef(const std::string& ref) {
307307
}
308308

309309
TableScanBuilder& TableScanBuilder::AsOfTime(int64_t timestamp_millis) {
310-
ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto time_point_ms,
311-
TimePointMsFromUnixMs(timestamp_millis));
310+
auto time_point_ms = TimePointMsFromUnixMs(timestamp_millis);
312311
ICEBERG_BUILDER_ASSIGN_OR_RETURN(
313312
auto snapshot_id, SnapshotUtil::SnapshotIdAsOfTime(*metadata_, time_point_ms));
314313
return UseSnapshot(snapshot_id);

‎src/iceberg/test/json_internal_test.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ TEST(JsonInternalTest, Snapshot) {
211211
Snapshot snapshot{.snapshot_id = 1234567890,
212212
.parent_snapshot_id = 9876543210,
213213
.sequence_number = 99,
214-
.timestamp_ms = TimePointMsFromUnixMs(1234567890123).value(),
214+
.timestamp_ms = TimePointMsFromUnixMs(1234567890123),
215215
.manifest_list = "/path/to/manifest_list",
216216
.summary = summary,
217217
.schema_id = 42};
@@ -403,7 +403,7 @@ TEST(JsonInternalTest, TableUpdateAddSnapshot) {
403403
Snapshot{.snapshot_id = 123456789,
404404
.parent_snapshot_id = 987654321,
405405
.sequence_number = 5,
406-
.timestamp_ms = TimePointMsFromUnixMs(1234567890000).value(),
406+
.timestamp_ms = TimePointMsFromUnixMs(1234567890000),
407407
.manifest_list = "/path/to/manifest-list.avro",
408408
.summary = {{SnapshotSummaryFields::kOperation, DataOperation::kAppend}},
409409
.schema_id = 1});

‎src/iceberg/test/metadata_io_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class MetadataIOTest : public TempFileTestBase {
7474
.snapshots = {std::make_shared<Snapshot>(Snapshot{
7575
.snapshot_id = 3051729675574597004,
7676
.sequence_number = 0,
77-
.timestamp_ms = TimePointMsFromUnixMs(1515100955770).value(),
77+
.timestamp_ms = TimePointMsFromUnixMs(1515100955770),
7878
.manifest_list = "s3://a/b/1.avro",
7979
.summary = {{"operation", "append"}},
8080
})},

‎src/iceberg/test/metadata_serde_test.cc‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ TEST(MetadataSerdeTest, DeserializeV1Valid) {
111111
.table_uuid = "d20125c8-7284-442c-9aea-15fee620737c",
112112
.location = "s3://bucket/test/location",
113113
.last_sequence_number = 0,
114-
.last_updated_ms = TimePointMsFromUnixMs(1602638573874).value(),
114+
.last_updated_ms = TimePointMsFromUnixMs(1602638573874),
115115
.last_column_id = 3,
116116
.schemas = {expected_schema},
117117
.current_schema_id = Schema::kInitialSchemaId,
@@ -170,7 +170,7 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) {
170170
auto expected_snapshot_1 = std::make_shared<Snapshot>(Snapshot{
171171
.snapshot_id = 3051729675574597004,
172172
.sequence_number = 0,
173-
.timestamp_ms = TimePointMsFromUnixMs(1515100955770).value(),
173+
.timestamp_ms = TimePointMsFromUnixMs(1515100955770),
174174
.manifest_list = "s3://a/b/1.avro",
175175
.summary = {{"operation", "append"}},
176176
});
@@ -179,7 +179,7 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) {
179179
.snapshot_id = 3055729675574597004,
180180
.parent_snapshot_id = 3051729675574597004,
181181
.sequence_number = 1,
182-
.timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(),
182+
.timestamp_ms = TimePointMsFromUnixMs(1555100955770),
183183
.manifest_list = "s3://a/b/2.avro",
184184
.summary = {{"operation", "append"}},
185185
.schema_id = 1,
@@ -190,7 +190,7 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) {
190190
.table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1",
191191
.location = "s3://bucket/test/location",
192192
.last_sequence_number = 34,
193-
.last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(),
193+
.last_updated_ms = TimePointMsFromUnixMs(1602638573590),
194194
.last_column_id = 3,
195195
.schemas = {expected_schema_1, expected_schema_2},
196196
.current_schema_id = 1,
@@ -200,10 +200,10 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) {
200200
.current_snapshot_id = 3055729675574597004,
201201
.snapshots = {expected_snapshot_1, expected_snapshot_2},
202202
.snapshot_log = {SnapshotLogEntry{
203-
.timestamp_ms = TimePointMsFromUnixMs(1515100955770).value(),
203+
.timestamp_ms = TimePointMsFromUnixMs(1515100955770),
204204
.snapshot_id = 3051729675574597004},
205205
SnapshotLogEntry{
206-
.timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(),
206+
.timestamp_ms = TimePointMsFromUnixMs(1555100955770),
207207
.snapshot_id = 3055729675574597004}},
208208
.sort_orders = {expected_sort_order},
209209
.default_sort_order_id = 3,
@@ -260,7 +260,7 @@ TEST(MetadataSerdeTest, DeserializeV2ValidMinimal) {
260260
.table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1",
261261
.location = "s3://bucket/test/location",
262262
.last_sequence_number = 34,
263-
.last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(),
263+
.last_updated_ms = TimePointMsFromUnixMs(1602638573590),
264264
.last_column_id = 3,
265265
.schemas = {expected_schema},
266266
.current_schema_id = 0,
@@ -298,7 +298,7 @@ TEST(MetadataSerdeTest, DeserializeStatisticsFiles) {
298298
auto expected_snapshot = std::make_shared<Snapshot>(Snapshot{
299299
.snapshot_id = 3055729675574597004,
300300
.sequence_number = 1,
301-
.timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(),
301+
.timestamp_ms = TimePointMsFromUnixMs(1555100955770),
302302
.manifest_list = "s3://a/b/2.avro",
303303
.summary = {{"operation", "append"}},
304304
.schema_id = 0,
@@ -326,7 +326,7 @@ TEST(MetadataSerdeTest, DeserializeStatisticsFiles) {
326326
.table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1",
327327
.location = "s3://bucket/test/location",
328328
.last_sequence_number = 34,
329-
.last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(),
329+
.last_updated_ms = TimePointMsFromUnixMs(1602638573590),
330330
.last_column_id = 3,
331331
.schemas = {expected_schema},
332332
.current_schema_id = 0,
@@ -361,7 +361,7 @@ TEST(MetadataSerdeTest, DeserializePartitionStatisticsFiles) {
361361
.table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1",
362362
.location = "s3://bucket/test/location",
363363
.last_sequence_number = 34,
364-
.last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(),
364+
.last_updated_ms = TimePointMsFromUnixMs(1602638573590),
365365
.last_column_id = 3,
366366
.schemas = {std::make_shared<Schema>(
367367
std::vector<SchemaField>{SchemaField(/*field_id=*/1, "x", int64(),
@@ -376,7 +376,7 @@ TEST(MetadataSerdeTest, DeserializePartitionStatisticsFiles) {
376376
.snapshots = {std::make_shared<Snapshot>(Snapshot{
377377
.snapshot_id = 3055729675574597004,
378378
.sequence_number = 1,
379-
.timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(),
379+
.timestamp_ms = TimePointMsFromUnixMs(1555100955770),
380380
.manifest_list = "s3://a/b/2.avro",
381381
.summary = {{"operation", "append"}},
382382
.schema_id = 0,

‎src/iceberg/test/snapshot_test.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ TEST_F(SnapshotTest, ConstructionAndFieldAccess) {
8585
Snapshot snapshot{.snapshot_id = 12345,
8686
.parent_snapshot_id = 54321,
8787
.sequence_number = 1,
88-
.timestamp_ms = TimePointMsFromUnixMs(1615569200000).value(),
88+
.timestamp_ms = TimePointMsFromUnixMs(1615569200000),
8989
.manifest_list = "s3://example/manifest_list.avro",
9090
.summary = summary1,
9191
.schema_id = 10};
@@ -107,13 +107,13 @@ TEST_F(SnapshotTest, ConstructionAndFieldAccess) {
107107

108108
TEST_F(SnapshotTest, EqualityComparison) {
109109
// Test the == and != operators
110-
Snapshot snapshot1(12345, {}, 1, TimePointMsFromUnixMs(1615569200000).value(),
110+
Snapshot snapshot1(12345, {}, 1, TimePointMsFromUnixMs(1615569200000),
111111
"s3://example/manifest_list.avro", summary1, {});
112112

113-
Snapshot snapshot2(12345, {}, 1, TimePointMsFromUnixMs(1615569200000).value(),
113+
Snapshot snapshot2(12345, {}, 1, TimePointMsFromUnixMs(1615569200000),
114114
"s3://example/manifest_list.avro", summary2, {});
115115

116-
Snapshot snapshot3(67890, {}, 1, TimePointMsFromUnixMs(1615569200000).value(),
116+
Snapshot snapshot3(67890, {}, 1, TimePointMsFromUnixMs(1615569200000),
117117
"s3://example/manifest_list.avro", summary3, {});
118118

119119
EXPECT_EQ(snapshot1, snapshot2);

‎src/iceberg/util/timepoint.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
namespace iceberg {
2727

28-
Result<TimePointMs> TimePointMsFromUnixMs(int64_t unix_ms) {
28+
TimePointMs TimePointMsFromUnixMs(int64_t unix_ms) {
2929
return TimePointMs{std::chrono::milliseconds(unix_ms)};
3030
}
3131

@@ -35,7 +35,7 @@ int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms) {
3535
.count();
3636
}
3737

38-
Result<TimePointNs> TimePointNsFromUnixNs(int64_t unix_ns) {
38+
TimePointNs TimePointNsFromUnixNs(int64_t unix_ns) {
3939
return TimePointNs{std::chrono::nanoseconds(unix_ns)};
4040
}
4141

‎src/iceberg/util/timepoint.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <chrono>
2323

2424
#include "iceberg/iceberg_export.h"
25-
#include "iceberg/result.h"
2625

2726
namespace iceberg {
2827

@@ -35,13 +34,13 @@ using TimePointNs =
3534
std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds>;
3635

3736
/// \brief Returns a TimePointMs from a Unix timestamp in milliseconds
38-
ICEBERG_EXPORT Result<TimePointMs> TimePointMsFromUnixMs(int64_t unix_ms);
37+
ICEBERG_EXPORT TimePointMs TimePointMsFromUnixMs(int64_t unix_ms);
3938

4039
/// \brief Returns a Unix timestamp in milliseconds from a TimePointMs
4140
ICEBERG_EXPORT int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms);
4241

4342
/// \brief Returns a TimePointNs from a Unix timestamp in nanoseconds
44-
ICEBERG_EXPORT Result<TimePointNs> TimePointNsFromUnixNs(int64_t unix_ns);
43+
ICEBERG_EXPORT TimePointNs TimePointNsFromUnixNs(int64_t unix_ns);
4544

4645
/// \brief Returns a Unix timestamp in nanoseconds from a TimePointNs
4746
ICEBERG_EXPORT int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns);

0 commit comments

Comments
 (0)