WZhuo commented on code in PR #801:
URL: https://github.com/apache/iceberg-cpp/pull/801#discussion_r3689086639
##########
src/iceberg/inspect/metadata_table.h:
##########
@@ -46,6 +60,28 @@ class ICEBERG_EXPORT MetadataTable {
virtual Kind kind() const noexcept = 0;
+ /// \brief Whether this metadata table supports time-travel queries.
+ ///
+ /// Time travel is supported for tables that read from a single snapshot's
+ /// manifests (e.g., Entries, Files, Manifests, Partitions). Tables that
+ /// scan all snapshots (All*) or return in-memory history (Snapshots,
+ /// History, Refs) do not support time travel.
Review Comment:
Fixed. The documentation now describes the metadata-table capability
directly, and time-travel behavior is represented by the separate
`TimeTravelMetadataTable` interface instead of listing unimplemented table
kinds.
##########
src/iceberg/test/metadata_table_test.cc:
##########
@@ -33,88 +33,40 @@
#include "iceberg/type.h"
namespace iceberg {
-namespace {
-
-std::shared_ptr<Schema> MakeSnapshotsSchema() {
- return std::make_shared<Schema>(std::vector<SchemaField>{
- SchemaField::MakeRequired(1, "committed_at", timestamp_tz()),
- SchemaField::MakeRequired(2, "snapshot_id", int64()),
- SchemaField::MakeOptional(3, "parent_id", int64()),
- SchemaField::MakeOptional(4, "operation", string()),
- SchemaField::MakeOptional(5, "manifest_list", string()),
- SchemaField::MakeOptional(
- 6, "summary",
- std::make_shared<MapType>(SchemaField::MakeRequired(7, "key",
string()),
- SchemaField::MakeRequired(8, "value",
string())))});
-}
-
-std::shared_ptr<Schema> MakeHistorySchema() {
- return std::make_shared<Schema>(std::vector<SchemaField>{
- SchemaField::MakeRequired(1, "made_current_at", timestamp_tz()),
- SchemaField::MakeRequired(2, "snapshot_id", int64()),
- SchemaField::MakeOptional(3, "parent_id", int64()),
- SchemaField::MakeRequired(4, "is_current_ancestor", boolean())});
-}
-
-} // namespace
class MetadataTableTest : public ::testing::Test {
protected:
void SetUp() override {
- io_ = std::make_shared<MockFileIO>();
- catalog_ = std::make_shared<MockCatalog>();
-
auto schema = std::make_shared<Schema>(
std::vector<SchemaField>{SchemaField::MakeRequired(1, "id", int64()),
SchemaField::MakeOptional(2, "name",
string())},
1);
- metadata_ = std::make_shared<TableMetadata>(
+ auto metadata = std::make_shared<TableMetadata>(
TableMetadata{.format_version = 2, .schemas = {schema},
.current_schema_id = 1});
Review Comment:
Fixed. This fixture now explicitly sets `current_snapshot_id =
kInvalidSnapshotId`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
