fix comment

Author: xiao.dong

src/iceberg/manifest_adapter.h

@@ -29,38 +29,39 @@
 
 namespace iceberg {
 
-// \brief Implemented by different versions with different schemas to
-// append a list of `ManifestEntry`s to an `ArrowArray`.
-class ManifestEntryAdapter {
+// \brief Base class of adapter for v1v2v3v4 metadata.
+class ICEBERG_EXPORT MetadataAdapter {
  public:
-  ManifestEntryAdapter() = default;
-  virtual ~ManifestEntryAdapter() = default;
+  MetadataAdapter() = default;
+  virtual ~MetadataAdapter() = default;
 
   virtual Status StartAppending() = 0;
-  virtual Status Append(const ManifestEntry& entry) = 0;
   virtual Result<ArrowArray> FinishAppending() = 0;
   int64_t size() const { return size_; }
 
  protected:
-  ArrowArray array_;  // array to append `ManifestEntry`s
+  ArrowArray array_;
   int64_t size_ = 0;
 };
 
+// \brief Implemented by different versions with different schemas to
+// append a list of `ManifestEntry`s to an `ArrowArray`.
+class ICEBERG_EXPORT ManifestEntryAdapter : public MetadataAdapter {
+ public:
+  ManifestEntryAdapter() = default;
+  virtual ~ManifestEntryAdapter() = default;
+
+  virtual Status Append(const ManifestEntry& entry) = 0;
+};
+
 // \brief Implemented by different versions with different schemas to
 // append a list of `ManifestFile`s to an `ArrowArray`.
-class ManifestFileAdapter {
+class ICEBERG_EXPORT ManifestFileAdapter : public MetadataAdapter {
  public:
   ManifestFileAdapter() = default;
   virtual ~ManifestFileAdapter() = default;
 
-  virtual Status StartAppending() = 0;
   virtual Status Append(const ManifestFile& file) = 0;
-  virtual Result<ArrowArray> FinishAppending() = 0;
-  int64_t size() const { return size_; }
-
- protected:
-  ArrowArray array_;  // array to append `ManifestFile`s
-  int64_t size_ = 0;
 };
 
 }  // namespace iceberg

src/iceberg/manifest_writer.cc

@@ -29,42 +29,29 @@
 
 namespace iceberg {
 
-/// \brief Write manifest files to a manifest list file.
-class ManifestWriterImpl : public ManifestWriter {
- public:
-  ManifestWriterImpl(std::unique_ptr<Writer> writer,
-                     std::unique_ptr<ManifestEntryAdapter> adapter)
-      : writer_(std::move(writer)), adapter_(std::move(adapter)) {}
-
-  Status Add(const ManifestEntry& entry) override {
-    if (adapter_->size() >= kBatchSize) {
-      ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
-      ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
-      ICEBERG_RETURN_UNEXPECTED(adapter_->StartAppending());
-    }
-    return adapter_->Append(entry);
+Status ManifestWriter::Add(const ManifestEntry& entry) {
+  if (adapter_->size() >= kBatchSize) {
+    ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
+    ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
+    ICEBERG_RETURN_UNEXPECTED(adapter_->StartAppending());
   }
+  return adapter_->Append(entry);
+}
 
-  Status AddAll(const std::vector<ManifestEntry>& entries) override {
-    for (const auto& entry : entries) {
-      ICEBERG_RETURN_UNEXPECTED(Add(entry));
-    }
-    return {};
+Status ManifestWriter::AddAll(const std::vector<ManifestEntry>& entries) {
+  for (const auto& entry : entries) {
+    ICEBERG_RETURN_UNEXPECTED(Add(entry));
   }
+  return {};
+}
 
-  Status Close() override {
-    if (adapter_->size() > 0) {
-      ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
-      ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
-    }
-    return {};
+Status ManifestWriter::Close() {
+  if (adapter_->size() > 0) {
+    ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
+    ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
   }
-
- private:
-  static constexpr int64_t kBatchSize = 1024;
-  std::unique_ptr<Writer> writer_;
-  std::unique_ptr<ManifestEntryAdapter> adapter_;
-};
+  return {};
+}
 
 Result<std::unique_ptr<Writer>> OpenFileWriter(std::string_view location,
                                                const std::shared_ptr<Schema> schema,
@@ -89,7 +76,7 @@ Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV1Writer(
   ICEBERG_ASSIGN_OR_RAISE(auto writer,
                           OpenFileWriter(manifest_location, schema, std::move(file_io)));
   auto adapter = std::make_unique<ManifestEntryAdapterV1>(snapshot_id, std::move(schema));
-  return std::make_unique<ManifestWriterImpl>(std::move(writer), std::move(adapter));
+  return std::make_unique<ManifestWriter>(std::move(writer), std::move(adapter));
 }
 
 Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV2Writer(
@@ -104,7 +91,7 @@ Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV2Writer(
   ICEBERG_ASSIGN_OR_RAISE(auto writer,
                           OpenFileWriter(manifest_location, schema, std::move(file_io)));
   auto adapter = std::make_unique<ManifestEntryAdapterV2>(snapshot_id, std::move(schema));
-  return std::make_unique<ManifestWriterImpl>(std::move(writer), std::move(adapter));
+  return std::make_unique<ManifestWriter>(std::move(writer), std::move(adapter));
 }
 
 Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV3Writer(
@@ -121,45 +108,32 @@ Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV3Writer(
                           OpenFileWriter(manifest_location, schema, std::move(file_io)));
   auto adapter = std::make_unique<ManifestEntryAdapterV3>(snapshot_id, first_row_id,
                                                           std::move(schema));
-  return std::make_unique<ManifestWriterImpl>(std::move(writer), std::move(adapter));
+  return std::make_unique<ManifestWriter>(std::move(writer), std::move(adapter));
 }
 
-/// \brief Write manifest files to a manifest list file.
-class ManifestListWriterImpl : public ManifestListWriter {
- public:
-  ManifestListWriterImpl(std::unique_ptr<Writer> writer,
-                         std::unique_ptr<ManifestFileAdapter> adapter)
-      : writer_(std::move(writer)), adapter_(std::move(adapter)) {}
-
-  Status Add(const ManifestFile& file) override {
-    if (adapter_->size() >= kBatchSize) {
-      ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
-      ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
-      ICEBERG_RETURN_UNEXPECTED(adapter_->StartAppending());
-    }
-    return adapter_->Append(file);
+Status ManifestListWriter::Add(const ManifestFile& file) {
+  if (adapter_->size() >= kBatchSize) {
+    ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
+    ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
+    ICEBERG_RETURN_UNEXPECTED(adapter_->StartAppending());
   }
+  return adapter_->Append(file);
+}
 
-  Status AddAll(const std::vector<ManifestFile>& files) override {
-    for (const auto& file : files) {
-      ICEBERG_RETURN_UNEXPECTED(Add(file));
-    }
-    return {};
+Status ManifestListWriter::AddAll(const std::vector<ManifestFile>& files) {
+  for (const auto& file : files) {
+    ICEBERG_RETURN_UNEXPECTED(Add(file));
   }
+  return {};
+}
 
-  Status Close() override {
-    if (adapter_->size() > 0) {
-      ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
-      ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
-    }
-    return {};
+Status ManifestListWriter::Close() {
+  if (adapter_->size() > 0) {
+    ICEBERG_ASSIGN_OR_RAISE(auto array, adapter_->FinishAppending());
+    ICEBERG_RETURN_UNEXPECTED(writer_->Write(array));
   }
-
- private:
-  static constexpr int64_t kBatchSize = 1024;
-  std::unique_ptr<Writer> writer_;
-  std::unique_ptr<ManifestFileAdapter> adapter_;
-};
+  return {};
+}
 
 Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV1Writer(
     int64_t snapshot_id, std::optional<int64_t> parent_snapshot_id,
@@ -172,7 +146,7 @@ Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV1Writer(
       auto writer, OpenFileWriter(manifest_list_location, schema, std::move(file_io)));
   auto adapter = std::make_unique<ManifestFileAdapterV1>(snapshot_id, parent_snapshot_id,
                                                          std::move(schema));
-  return std::make_unique<ManifestListWriterImpl>(std::move(writer), std::move(adapter));
+  return std::make_unique<ManifestListWriter>(std::move(writer), std::move(adapter));
 }
 
 Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV2Writer(
@@ -187,7 +161,7 @@ Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV2Writer(
       auto writer, OpenFileWriter(manifest_list_location, schema, std::move(file_io)));
   auto adapter = std::make_unique<ManifestFileAdapterV2>(
       snapshot_id, parent_snapshot_id, sequence_number, std::move(schema));
-  return std::make_unique<ManifestListWriterImpl>(std::move(writer), std::move(adapter));
+  return std::make_unique<ManifestListWriter>(std::move(writer), std::move(adapter));
 }
 
 Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV3Writer(
@@ -202,7 +176,7 @@ Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV3Writer(
       auto writer, OpenFileWriter(manifest_list_location, schema, std::move(file_io)));
   auto adapter = std::make_unique<ManifestFileAdapterV3>(
       snapshot_id, parent_snapshot_id, sequence_number, first_row_id, std::move(schema));
-  return std::make_unique<ManifestListWriterImpl>(std::move(writer), std::move(adapter));
+  return std::make_unique<ManifestListWriter>(std::move(writer), std::move(adapter));
 }
 
 }  // namespace iceberg

src/iceberg/manifest_writer.h

@@ -27,27 +27,32 @@
 
 #include "iceberg/file_writer.h"
 #include "iceberg/iceberg_export.h"
+#include "iceberg/manifest_adapter.h"
 #include "iceberg/type_fwd.h"
 
 namespace iceberg {
 
 /// \brief Write manifest entries to a manifest file.
 class ICEBERG_EXPORT ManifestWriter {
  public:
+  ManifestWriter(std::unique_ptr<Writer> writer,
+                 std::unique_ptr<ManifestEntryAdapter> adapter)
+      : writer_(std::move(writer)), adapter_(std::move(adapter)) {}
+
   virtual ~ManifestWriter() = default;
 
   /// \brief Write manifest entry to file.
   /// \param entry Manifest entry to write.
   /// \return Status::OK() if entry was written successfully
-  virtual Status Add(const ManifestEntry& entry) = 0;
+  Status Add(const ManifestEntry& entry);
 
   /// \brief Write manifest entries to file.
   /// \param entries Manifest entries to write.
   /// \return Status::OK() if all entries were written successfully
-  virtual Status AddAll(const std::vector<ManifestEntry>& entries) = 0;
+  Status AddAll(const std::vector<ManifestEntry>& entries);
 
   /// \brief Close writer and flush to storage.
-  virtual Status Close() = 0;
+  Status Close();
 
   /// \brief Creates a writer for a manifest file.
   /// \param snapshot_id ID of the snapshot.
@@ -77,25 +82,34 @@ class ICEBERG_EXPORT ManifestWriter {
       std::optional<int64_t> snapshot_id, std::optional<int64_t> first_row_id,
       std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
       std::shared_ptr<Schema> partition_schema);
+
+ private:
+  static constexpr int64_t kBatchSize = 1024;
+  std::unique_ptr<Writer> writer_;
+  std::unique_ptr<ManifestEntryAdapter> adapter_;
 };
 
 /// \brief Write manifest files to a manifest list file.
 class ICEBERG_EXPORT ManifestListWriter {
  public:
+  ManifestListWriter(std::unique_ptr<Writer> writer,
+                     std::unique_ptr<ManifestFileAdapter> adapter)
+      : writer_(std::move(writer)), adapter_(std::move(adapter)) {}
+
   virtual ~ManifestListWriter() = default;
 
   /// \brief Write manifest file to manifest list file.
   /// \param file Manifest file to write.
   /// \return Status::OK() if file was written successfully
-  virtual Status Add(const ManifestFile& file) = 0;
+  Status Add(const ManifestFile& file);
 
   /// \brief Write manifest file list to manifest list file.
   /// \param files Manifest file list to write.
   /// \return Status::OK() if all files were written successfully
-  virtual Status AddAll(const std::vector<ManifestFile>& files) = 0;
+  Status AddAll(const std::vector<ManifestFile>& files);
 
   /// \brief Close writer and flush to storage.
-  virtual Status Close() = 0;
+  Status Close();
 
   /// \brief Creates a writer for the v1 manifest list.
   /// \param snapshot_id ID of the snapshot.
@@ -131,6 +145,11 @@ class ICEBERG_EXPORT ManifestListWriter {
       int64_t snapshot_id, std::optional<int64_t> parent_snapshot_id,
       int64_t sequence_number, std::optional<int64_t> first_row_id,
       std::string_view manifest_list_location, std::shared_ptr<FileIO> file_io);
+
+ private:
+  static constexpr int64_t kBatchSize = 1024;
+  std::unique_ptr<Writer> writer_;
+  std::unique_ptr<ManifestFileAdapter> adapter_;
 };
 
 }  // namespace iceberg