diff --git a/src/iceberg/json_internal.cc b/src/iceberg/json_internal.cc index 669d0b50b..496205049 100644 --- a/src/iceberg/json_internal.cc +++ b/src/iceberg/json_internal.cc @@ -645,9 +645,8 @@ Result> SnapshotFromJson(const nlohmann::json& json) { ICEBERG_ASSIGN_OR_RAISE(auto snapshot_id, GetJsonValue(json, kSnapshotId)); ICEBERG_ASSIGN_OR_RAISE(auto sequence_number, GetJsonValueOptional(json, kSequenceNumber)); - ICEBERG_ASSIGN_OR_RAISE( - auto timestamp_ms, - GetJsonValue(json, kTimestampMs).and_then(TimePointMsFromUnixMs)); + ICEBERG_ASSIGN_OR_RAISE(auto unix_ms, GetJsonValue(json, kTimestampMs)); + auto timestamp_ms = TimePointMsFromUnixMs(unix_ms); ICEBERG_ASSIGN_OR_RAISE(auto manifest_list, GetJsonValue(json, kManifestList)); @@ -781,9 +780,8 @@ nlohmann::json ToJson(const SnapshotLogEntry& snapshot_log_entry) { Result SnapshotLogEntryFromJson(const nlohmann::json& json) { SnapshotLogEntry snapshot_log_entry; - ICEBERG_ASSIGN_OR_RAISE( - snapshot_log_entry.timestamp_ms, - GetJsonValue(json, kTimestampMs).and_then(TimePointMsFromUnixMs)); + ICEBERG_ASSIGN_OR_RAISE(auto unix_ms, GetJsonValue(json, kTimestampMs)); + snapshot_log_entry.timestamp_ms = TimePointMsFromUnixMs(unix_ms); ICEBERG_ASSIGN_OR_RAISE(snapshot_log_entry.snapshot_id, GetJsonValue(json, kSnapshotId)); return snapshot_log_entry; @@ -798,9 +796,8 @@ nlohmann::json ToJson(const MetadataLogEntry& metadata_log_entry) { Result MetadataLogEntryFromJson(const nlohmann::json& json) { MetadataLogEntry metadata_log_entry; - ICEBERG_ASSIGN_OR_RAISE( - metadata_log_entry.timestamp_ms, - GetJsonValue(json, kTimestampMs).and_then(TimePointMsFromUnixMs)); + ICEBERG_ASSIGN_OR_RAISE(auto unix_ms, GetJsonValue(json, kTimestampMs)); + metadata_log_entry.timestamp_ms = TimePointMsFromUnixMs(unix_ms); ICEBERG_ASSIGN_OR_RAISE(metadata_log_entry.metadata_file, GetJsonValue(json, kMetadataFile)); return metadata_log_entry; diff --git a/src/iceberg/table_scan.cc b/src/iceberg/table_scan.cc index 124102705..05d44d905 100644 --- a/src/iceberg/table_scan.cc +++ b/src/iceberg/table_scan.cc @@ -307,8 +307,7 @@ TableScanBuilder& TableScanBuilder::UseRef(const std::string& ref) { } TableScanBuilder& TableScanBuilder::AsOfTime(int64_t timestamp_millis) { - ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto time_point_ms, - TimePointMsFromUnixMs(timestamp_millis)); + auto time_point_ms = TimePointMsFromUnixMs(timestamp_millis); ICEBERG_BUILDER_ASSIGN_OR_RETURN( auto snapshot_id, SnapshotUtil::SnapshotIdAsOfTime(*metadata_, time_point_ms)); return UseSnapshot(snapshot_id); diff --git a/src/iceberg/test/json_internal_test.cc b/src/iceberg/test/json_internal_test.cc index f88527ff4..4707d740c 100644 --- a/src/iceberg/test/json_internal_test.cc +++ b/src/iceberg/test/json_internal_test.cc @@ -211,7 +211,7 @@ TEST(JsonInternalTest, Snapshot) { Snapshot snapshot{.snapshot_id = 1234567890, .parent_snapshot_id = 9876543210, .sequence_number = 99, - .timestamp_ms = TimePointMsFromUnixMs(1234567890123).value(), + .timestamp_ms = TimePointMsFromUnixMs(1234567890123), .manifest_list = "/path/to/manifest_list", .summary = summary, .schema_id = 42}; @@ -403,7 +403,7 @@ TEST(JsonInternalTest, TableUpdateAddSnapshot) { Snapshot{.snapshot_id = 123456789, .parent_snapshot_id = 987654321, .sequence_number = 5, - .timestamp_ms = TimePointMsFromUnixMs(1234567890000).value(), + .timestamp_ms = TimePointMsFromUnixMs(1234567890000), .manifest_list = "/path/to/manifest-list.avro", .summary = {{SnapshotSummaryFields::kOperation, DataOperation::kAppend}}, .schema_id = 1}); diff --git a/src/iceberg/test/metadata_io_test.cc b/src/iceberg/test/metadata_io_test.cc index 7b11cf47e..16661235c 100644 --- a/src/iceberg/test/metadata_io_test.cc +++ b/src/iceberg/test/metadata_io_test.cc @@ -74,7 +74,7 @@ class MetadataIOTest : public TempFileTestBase { .snapshots = {std::make_shared(Snapshot{ .snapshot_id = 3051729675574597004, .sequence_number = 0, - .timestamp_ms = TimePointMsFromUnixMs(1515100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1515100955770), .manifest_list = "s3://a/b/1.avro", .summary = {{"operation", "append"}}, })}, diff --git a/src/iceberg/test/metadata_serde_test.cc b/src/iceberg/test/metadata_serde_test.cc index 3668817ed..0d3b5959b 100644 --- a/src/iceberg/test/metadata_serde_test.cc +++ b/src/iceberg/test/metadata_serde_test.cc @@ -111,7 +111,7 @@ TEST(MetadataSerdeTest, DeserializeV1Valid) { .table_uuid = "d20125c8-7284-442c-9aea-15fee620737c", .location = "s3://bucket/test/location", .last_sequence_number = 0, - .last_updated_ms = TimePointMsFromUnixMs(1602638573874).value(), + .last_updated_ms = TimePointMsFromUnixMs(1602638573874), .last_column_id = 3, .schemas = {expected_schema}, .current_schema_id = Schema::kInitialSchemaId, @@ -170,7 +170,7 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) { auto expected_snapshot_1 = std::make_shared(Snapshot{ .snapshot_id = 3051729675574597004, .sequence_number = 0, - .timestamp_ms = TimePointMsFromUnixMs(1515100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1515100955770), .manifest_list = "s3://a/b/1.avro", .summary = {{"operation", "append"}}, }); @@ -179,7 +179,7 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) { .snapshot_id = 3055729675574597004, .parent_snapshot_id = 3051729675574597004, .sequence_number = 1, - .timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1555100955770), .manifest_list = "s3://a/b/2.avro", .summary = {{"operation", "append"}}, .schema_id = 1, @@ -190,7 +190,7 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) { .table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1", .location = "s3://bucket/test/location", .last_sequence_number = 34, - .last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(), + .last_updated_ms = TimePointMsFromUnixMs(1602638573590), .last_column_id = 3, .schemas = {expected_schema_1, expected_schema_2}, .current_schema_id = 1, @@ -200,10 +200,10 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) { .current_snapshot_id = 3055729675574597004, .snapshots = {expected_snapshot_1, expected_snapshot_2}, .snapshot_log = {SnapshotLogEntry{ - .timestamp_ms = TimePointMsFromUnixMs(1515100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1515100955770), .snapshot_id = 3051729675574597004}, SnapshotLogEntry{ - .timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1555100955770), .snapshot_id = 3055729675574597004}}, .sort_orders = {expected_sort_order}, .default_sort_order_id = 3, @@ -260,7 +260,7 @@ TEST(MetadataSerdeTest, DeserializeV2ValidMinimal) { .table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1", .location = "s3://bucket/test/location", .last_sequence_number = 34, - .last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(), + .last_updated_ms = TimePointMsFromUnixMs(1602638573590), .last_column_id = 3, .schemas = {expected_schema}, .current_schema_id = 0, @@ -298,7 +298,7 @@ TEST(MetadataSerdeTest, DeserializeStatisticsFiles) { auto expected_snapshot = std::make_shared(Snapshot{ .snapshot_id = 3055729675574597004, .sequence_number = 1, - .timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1555100955770), .manifest_list = "s3://a/b/2.avro", .summary = {{"operation", "append"}}, .schema_id = 0, @@ -326,7 +326,7 @@ TEST(MetadataSerdeTest, DeserializeStatisticsFiles) { .table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1", .location = "s3://bucket/test/location", .last_sequence_number = 34, - .last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(), + .last_updated_ms = TimePointMsFromUnixMs(1602638573590), .last_column_id = 3, .schemas = {expected_schema}, .current_schema_id = 0, @@ -361,7 +361,7 @@ TEST(MetadataSerdeTest, DeserializePartitionStatisticsFiles) { .table_uuid = "9c12d441-03fe-4693-9a96-a0705ddf69c1", .location = "s3://bucket/test/location", .last_sequence_number = 34, - .last_updated_ms = TimePointMsFromUnixMs(1602638573590).value(), + .last_updated_ms = TimePointMsFromUnixMs(1602638573590), .last_column_id = 3, .schemas = {std::make_shared( std::vector{SchemaField(/*field_id=*/1, "x", int64(), @@ -376,7 +376,7 @@ TEST(MetadataSerdeTest, DeserializePartitionStatisticsFiles) { .snapshots = {std::make_shared(Snapshot{ .snapshot_id = 3055729675574597004, .sequence_number = 1, - .timestamp_ms = TimePointMsFromUnixMs(1555100955770).value(), + .timestamp_ms = TimePointMsFromUnixMs(1555100955770), .manifest_list = "s3://a/b/2.avro", .summary = {{"operation", "append"}}, .schema_id = 0, diff --git a/src/iceberg/test/snapshot_test.cc b/src/iceberg/test/snapshot_test.cc index a3c28f89f..5d6016484 100644 --- a/src/iceberg/test/snapshot_test.cc +++ b/src/iceberg/test/snapshot_test.cc @@ -85,7 +85,7 @@ TEST_F(SnapshotTest, ConstructionAndFieldAccess) { Snapshot snapshot{.snapshot_id = 12345, .parent_snapshot_id = 54321, .sequence_number = 1, - .timestamp_ms = TimePointMsFromUnixMs(1615569200000).value(), + .timestamp_ms = TimePointMsFromUnixMs(1615569200000), .manifest_list = "s3://example/manifest_list.avro", .summary = summary1, .schema_id = 10}; @@ -107,13 +107,13 @@ TEST_F(SnapshotTest, ConstructionAndFieldAccess) { TEST_F(SnapshotTest, EqualityComparison) { // Test the == and != operators - Snapshot snapshot1(12345, {}, 1, TimePointMsFromUnixMs(1615569200000).value(), + Snapshot snapshot1(12345, {}, 1, TimePointMsFromUnixMs(1615569200000), "s3://example/manifest_list.avro", summary1, {}); - Snapshot snapshot2(12345, {}, 1, TimePointMsFromUnixMs(1615569200000).value(), + Snapshot snapshot2(12345, {}, 1, TimePointMsFromUnixMs(1615569200000), "s3://example/manifest_list.avro", summary2, {}); - Snapshot snapshot3(67890, {}, 1, TimePointMsFromUnixMs(1615569200000).value(), + Snapshot snapshot3(67890, {}, 1, TimePointMsFromUnixMs(1615569200000), "s3://example/manifest_list.avro", summary3, {}); EXPECT_EQ(snapshot1, snapshot2); diff --git a/src/iceberg/util/timepoint.cc b/src/iceberg/util/timepoint.cc index 0381e90a6..112825f25 100644 --- a/src/iceberg/util/timepoint.cc +++ b/src/iceberg/util/timepoint.cc @@ -25,7 +25,7 @@ namespace iceberg { -Result TimePointMsFromUnixMs(int64_t unix_ms) { +TimePointMs TimePointMsFromUnixMs(int64_t unix_ms) { return TimePointMs{std::chrono::milliseconds(unix_ms)}; } @@ -35,7 +35,7 @@ int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms) { .count(); } -Result TimePointNsFromUnixNs(int64_t unix_ns) { +TimePointNs TimePointNsFromUnixNs(int64_t unix_ns) { return TimePointNs{std::chrono::nanoseconds(unix_ns)}; } diff --git a/src/iceberg/util/timepoint.h b/src/iceberg/util/timepoint.h index 6052c94ae..19b684c01 100644 --- a/src/iceberg/util/timepoint.h +++ b/src/iceberg/util/timepoint.h @@ -22,7 +22,6 @@ #include #include "iceberg/iceberg_export.h" -#include "iceberg/result.h" namespace iceberg { @@ -35,13 +34,13 @@ using TimePointNs = std::chrono::time_point; /// \brief Returns a TimePointMs from a Unix timestamp in milliseconds -ICEBERG_EXPORT Result TimePointMsFromUnixMs(int64_t unix_ms); +ICEBERG_EXPORT TimePointMs TimePointMsFromUnixMs(int64_t unix_ms); /// \brief Returns a Unix timestamp in milliseconds from a TimePointMs ICEBERG_EXPORT int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms); /// \brief Returns a TimePointNs from a Unix timestamp in nanoseconds -ICEBERG_EXPORT Result TimePointNsFromUnixNs(int64_t unix_ns); +ICEBERG_EXPORT TimePointNs TimePointNsFromUnixNs(int64_t unix_ns); /// \brief Returns a Unix timestamp in nanoseconds from a TimePointNs ICEBERG_EXPORT int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns);