fix comments

Author: gty404

src/iceberg/metadata_update.h

@@ -109,10 +109,10 @@ class ICEBERG_EXPORT UpgradeFormatVersion : public MetadataUpdate {
 /// \brief Represents adding a new schema to the table
 class ICEBERG_EXPORT AddSchema : public MetadataUpdate {
  public:
-  explicit AddSchema(std::shared_ptr<iceberg::Schema> schema, int32_t last_column_id)
+  explicit AddSchema(std::shared_ptr<Schema> schema, int32_t last_column_id)
       : schema_(std::move(schema)), last_column_id_(last_column_id) {}
 
-  const std::shared_ptr<iceberg::Schema>& schema() const { return schema_; }
+  const std::shared_ptr<Schema>& schema() const { return schema_; }
 
   int32_t last_column_id() const { return last_column_id_; }
 
@@ -125,7 +125,7 @@ class ICEBERG_EXPORT AddSchema : public MetadataUpdate {
   void GenerateRequirements(UpdateRequirementsContext& context) const override;
 
  private:
-  std::shared_ptr<iceberg::Schema> schema_;
+  std::shared_ptr<Schema> schema_;
   int32_t last_column_id_;
 };
 
@@ -151,10 +151,10 @@ class ICEBERG_EXPORT SetCurrentSchema : public MetadataUpdate {
 /// \brief Represents adding a new partition spec to the table
 class ICEBERG_EXPORT AddPartitionSpec : public MetadataUpdate {
  public:
-  explicit AddPartitionSpec(std::shared_ptr<iceberg::PartitionSpec> spec)
+  explicit AddPartitionSpec(std::shared_ptr<PartitionSpec> spec)
       : spec_(std::move(spec)) {}
 
-  const std::shared_ptr<iceberg::PartitionSpec>& spec() const { return spec_; }
+  const std::shared_ptr<PartitionSpec>& spec() const { return spec_; }
 
   std::unique_ptr<MetadataUpdate> Clone() const override {
     return std::make_unique<AddPartitionSpec>(spec_);
@@ -165,7 +165,7 @@ class ICEBERG_EXPORT AddPartitionSpec : public MetadataUpdate {
   void GenerateRequirements(UpdateRequirementsContext& context) const override;
 
  private:
-  std::shared_ptr<iceberg::PartitionSpec> spec_;
+  std::shared_ptr<PartitionSpec> spec_;
 };
 
 /// \brief Represents setting the default partition spec
@@ -230,10 +230,10 @@ class ICEBERG_EXPORT RemoveSchemas : public MetadataUpdate {
 /// \brief Represents adding a new sort order to the table
 class ICEBERG_EXPORT AddSortOrder : public MetadataUpdate {
  public:
-  explicit AddSortOrder(std::shared_ptr<iceberg::SortOrder> sort_order)
+  explicit AddSortOrder(std::shared_ptr<SortOrder> sort_order)
       : sort_order_(std::move(sort_order)) {}
 
-  const std::shared_ptr<iceberg::SortOrder>& sort_order() const { return sort_order_; }
+  const std::shared_ptr<SortOrder>& sort_order() const { return sort_order_; }
 
   std::unique_ptr<MetadataUpdate> Clone() const override {
     return std::make_unique<AddSortOrder>(sort_order_);
@@ -244,7 +244,7 @@ class ICEBERG_EXPORT AddSortOrder : public MetadataUpdate {
   void GenerateRequirements(UpdateRequirementsContext& context) const override;
 
  private:
-  std::shared_ptr<iceberg::SortOrder> sort_order_;
+  std::shared_ptr<SortOrder> sort_order_;
 };
 
 /// \brief Represents setting the default sort order
@@ -269,10 +269,10 @@ class ICEBERG_EXPORT SetDefaultSortOrder : public MetadataUpdate {
 /// \brief Represents adding a snapshot to the table
 class ICEBERG_EXPORT AddSnapshot : public MetadataUpdate {
  public:
-  explicit AddSnapshot(std::shared_ptr<iceberg::Snapshot> snapshot)
+  explicit AddSnapshot(std::shared_ptr<Snapshot> snapshot)
       : snapshot_(std::move(snapshot)) {}
 
-  const std::shared_ptr<iceberg::Snapshot>& snapshot() const { return snapshot_; }
+  const std::shared_ptr<Snapshot>& snapshot() const { return snapshot_; }
 
   std::unique_ptr<MetadataUpdate> Clone() const override {
     return std::make_unique<AddSnapshot>(snapshot_);
@@ -283,7 +283,7 @@ class ICEBERG_EXPORT AddSnapshot : public MetadataUpdate {
   void GenerateRequirements(UpdateRequirementsContext& context) const override;
 
  private:
-  std::shared_ptr<iceberg::Snapshot> snapshot_;
+  std::shared_ptr<Snapshot> snapshot_;
 };
 
 /// \brief Represents removing snapshots from the table

src/iceberg/table_metadata.cc

@@ -27,6 +27,7 @@
 
 #include "iceberg/file_io.h"
 #include "iceberg/json_internal.h"
+#include "iceberg/metadata_update.h"
 #include "iceberg/partition_spec.h"
 #include "iceberg/result.h"
 #include "iceberg/schema.h"
@@ -201,47 +202,38 @@ Status TableMetadataUtil::Write(FileIO& io, const std::string& location,
 
 struct TableMetadataBuilder::Impl {
   // Base metadata (if building from existing metadata)
-  std::shared_ptr<const TableMetadata> base;
+  const TableMetadata* base{nullptr};
 
   // Mutable fields that will be used to build the final TableMetadata
   int8_t format_version;
   std::string table_uuid;
   std::string location;
-  int64_t last_sequence_number;
-  TimePointMs last_updated_ms;
-  int32_t last_column_id;
+  int64_t last_sequence_number{TableMetadata::kInitialSequenceNumber};
+  TimePointMs last_updated_ms{std::chrono::milliseconds(0)};
+  int32_t last_column_id{0};
   std::vector<std::shared_ptr<Schema>> schemas;
   std::optional<int32_t> current_schema_id;
   std::vector<std::shared_ptr<PartitionSpec>> partition_specs;
-  int32_t default_spec_id;
-  int32_t last_partition_id;
+  int32_t default_spec_id{0};
+  int32_t last_partition_id{0};
   std::unordered_map<std::string, std::string> properties;
-  int64_t current_snapshot_id;
+  int64_t current_snapshot_id{-1};
   std::vector<std::shared_ptr<Snapshot>> snapshots;
   std::vector<SnapshotLogEntry> snapshot_log;
   std::vector<MetadataLogEntry> metadata_log;
   std::vector<std::shared_ptr<SortOrder>> sort_orders;
-  int32_t default_sort_order_id;
+  int32_t default_sort_order_id{0};
   std::unordered_map<std::string, std::shared_ptr<SnapshotRef>> refs;
   std::vector<std::shared_ptr<StatisticsFile>> statistics;
   std::vector<std::shared_ptr<PartitionStatisticsFile>> partition_statistics;
-  int64_t next_row_id;
+  int64_t next_row_id{TableMetadata::kInitialRowId};
 
   // List of changes (MetadataUpdate objects)
   std::vector<std::unique_ptr<MetadataUpdate>> changes;
 
-  explicit Impl(int8_t fmt_version)
-      : format_version(fmt_version),
-        last_sequence_number(TableMetadata::kInitialSequenceNumber),
-        last_updated_ms(std::chrono::milliseconds(0)),
-        last_column_id(0),
-        default_spec_id(0),
-        last_partition_id(0),
-        current_snapshot_id(-1),
-        default_sort_order_id(0),
-        next_row_id(TableMetadata::kInitialRowId) {}
-
-  explicit Impl(const std::shared_ptr<const TableMetadata>& base_metadata)
+  explicit Impl(int8_t fmt_version) : format_version(fmt_version) {}
+
+  explicit Impl(const TableMetadata* base_metadata)
       : base(base_metadata),
         format_version(base_metadata->format_version),
         table_uuid(base_metadata->table_uuid),
@@ -270,7 +262,7 @@ struct TableMetadataBuilder::Impl {
 TableMetadataBuilder::TableMetadataBuilder(int8_t format_version)
     : impl_(std::make_unique<Impl>(format_version)) {}
 
-TableMetadataBuilder::TableMetadataBuilder(std::shared_ptr<const TableMetadata> base)
+TableMetadataBuilder::TableMetadataBuilder(const TableMetadata* base)
     : impl_(std::make_unique<Impl>(base)) {}
 
 TableMetadataBuilder::~TableMetadataBuilder() = default;
@@ -280,21 +272,23 @@ TableMetadataBuilder::TableMetadataBuilder(TableMetadataBuilder&&) noexcept = de
 TableMetadataBuilder& TableMetadataBuilder::operator=(TableMetadataBuilder&&) noexcept =
     default;
 
-TableMetadataBuilder TableMetadataBuilder::BuildFromEmpty(int8_t format_version) {
-  return TableMetadataBuilder(format_version);
+std::unique_ptr<TableMetadataBuilder> TableMetadataBuilder::BuildFromEmpty(
+    int8_t format_version) {
+  return std::unique_ptr<TableMetadataBuilder>(
+      new TableMetadataBuilder(format_version));  // NOLINT
 }
 
-TableMetadataBuilder TableMetadataBuilder::BuildFrom(
-    const std::shared_ptr<const TableMetadata>& base) {
-  return TableMetadataBuilder(base);
+std::unique_ptr<TableMetadataBuilder> TableMetadataBuilder::BuildFrom(
+    const TableMetadata* base) {
+  return std::unique_ptr<TableMetadataBuilder>(new TableMetadataBuilder(base));  // NOLINT
 }
 
 TableMetadataBuilder& TableMetadataBuilder::AssignUUID() {
   // TODO(gty404): Implement
   return *this;
 }
 
-TableMetadataBuilder& TableMetadataBuilder::AssignUUID(const std::string& uuid) {
+TableMetadataBuilder& TableMetadataBuilder::AssignUUID(std::string_view uuid) {
   // TODO(gty404): Implement
   return *this;
 }

src/iceberg/table_metadata.h

@@ -29,7 +29,6 @@
 #include <vector>
 
 #include "iceberg/iceberg_export.h"
-#include "iceberg/metadata_update.h"
 #include "iceberg/type_fwd.h"
 #include "iceberg/util/timepoint.h"
 
@@ -160,14 +159,14 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   ///
   /// \param format_version The format version for the table
   /// \return A new TableMetadataBuilder instance
-  static TableMetadataBuilder BuildFromEmpty(
+  static std::unique_ptr<TableMetadataBuilder> BuildFromEmpty(
       int8_t format_version = TableMetadata::kDefaultTableFormatVersion);
 
   /// \brief Create a builder from existing table metadata
   ///
   /// \param base The base table metadata to build from
   /// \return A new TableMetadataBuilder instance initialized with base metadata
-  static TableMetadataBuilder BuildFrom(const std::shared_ptr<const TableMetadata>& base);
+  static std::unique_ptr<TableMetadataBuilder> BuildFrom(const TableMetadata* base);
 
   /// \brief Assign a UUID to the table
   ///
@@ -179,7 +178,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   ///
   /// \param uuid The UUID string to assign
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& AssignUUID(const std::string& uuid);
+  TableMetadataBuilder& AssignUUID(std::string_view uuid);
 
   /// \brief Upgrade the format version of the table
   ///
@@ -192,7 +191,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   /// \param schema The schema to set as current
   /// \param new_last_column_id The highest column ID in the schema
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& SetCurrentSchema(std::shared_ptr<iceberg::Schema> schema,
+  TableMetadataBuilder& SetCurrentSchema(std::shared_ptr<Schema> schema,
                                          int32_t new_last_column_id);
 
   /// \brief Set the current schema by schema ID
@@ -205,14 +204,13 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   ///
   /// \param schema The schema to add
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& AddSchema(std::shared_ptr<iceberg::Schema> schema);
+  TableMetadataBuilder& AddSchema(std::shared_ptr<Schema> schema);
 
   /// \brief Set the default partition spec for the table
   ///
   /// \param spec The partition spec to set as default
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& SetDefaultPartitionSpec(
-      std::shared_ptr<iceberg::PartitionSpec> spec);
+  TableMetadataBuilder& SetDefaultPartitionSpec(std::shared_ptr<PartitionSpec> spec);
 
   /// \brief Set the default partition spec by spec ID
   ///
@@ -224,7 +222,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   ///
   /// \param spec The partition spec to add
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& AddPartitionSpec(std::shared_ptr<iceberg::PartitionSpec> spec);
+  TableMetadataBuilder& AddPartitionSpec(std::shared_ptr<PartitionSpec> spec);
 
   /// \brief Remove partition specs from the table
   ///
@@ -242,7 +240,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   ///
   /// \param order The sort order to set as default
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& SetDefaultSortOrder(std::shared_ptr<iceberg::SortOrder> order);
+  TableMetadataBuilder& SetDefaultSortOrder(std::shared_ptr<SortOrder> order);
 
   /// \brief Set the default sort order by order ID
   ///
@@ -254,13 +252,13 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   ///
   /// \param order The sort order to add
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& AddSortOrder(std::shared_ptr<iceberg::SortOrder> order);
+  TableMetadataBuilder& AddSortOrder(std::shared_ptr<SortOrder> order);
 
   /// \brief Add a snapshot to the table
   ///
   /// \param snapshot The snapshot to add
   /// \return Reference to this builder for method chaining
-  TableMetadataBuilder& AddSnapshot(std::shared_ptr<iceberg::Snapshot> snapshot);
+  TableMetadataBuilder& AddSnapshot(std::shared_ptr<Snapshot> snapshot);
 
   /// \brief Set a branch to point to a specific snapshot
   ///
@@ -335,7 +333,7 @@ class ICEBERG_EXPORT TableMetadataBuilder {
   explicit TableMetadataBuilder(int8_t format_version);
 
   /// \brief Private constructor for building from existing metadata
-  explicit TableMetadataBuilder(std::shared_ptr<const TableMetadata> base);
+  explicit TableMetadataBuilder(const TableMetadata* base);
 
   /// Internal state members
   struct Impl;

src/iceberg/update_requirement.cc

@@ -19,6 +19,7 @@
 
 #include "iceberg/update_requirement.h"
 
+#include "iceberg/snapshot.h"
 #include "iceberg/table_metadata.h"
 #include "iceberg/util/string_util.h"