fix: review comments & make the ctors in manifest_writer.h private

Author: zhjwpku

src/iceberg/manifest/manifest_writer.cc

@@ -294,8 +294,8 @@ Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV1Writer(
       auto writer,
       OpenFileWriter(manifest_location, std::move(schema), std::move(file_io),
                      adapter->metadata(), "manifest_entry"));
-  return std::make_unique<ManifestWriter>(std::move(writer), std::move(adapter),
-                                          manifest_location, std::nullopt);
+  return std::unique_ptr<ManifestWriter>(new ManifestWriter(
+      std::move(writer), std::move(adapter), manifest_location, std::nullopt));
 }
 
 Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV2Writer(
@@ -324,8 +324,8 @@ Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV2Writer(
       auto writer,
       OpenFileWriter(manifest_location, std::move(schema), std::move(file_io),
                      adapter->metadata(), "manifest_entry"));
-  return std::make_unique<ManifestWriter>(std::move(writer), std::move(adapter),
-                                          manifest_location, std::nullopt);
+  return std::unique_ptr<ManifestWriter>(new ManifestWriter(
+      std::move(writer), std::move(adapter), manifest_location, std::nullopt));
 }
 
 Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV3Writer(
@@ -356,8 +356,8 @@ Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeV3Writer(
       auto writer,
       OpenFileWriter(manifest_location, std::move(schema), std::move(file_io),
                      adapter->metadata(), "manifest_entry"));
-  return std::make_unique<ManifestWriter>(std::move(writer), std::move(adapter),
-                                          manifest_location, first_row_id);
+  return std::unique_ptr<ManifestWriter>(new ManifestWriter(
+      std::move(writer), std::move(adapter), manifest_location, first_row_id));
 }
 
 ManifestListWriter::ManifestListWriter(std::unique_ptr<Writer> writer,
@@ -406,7 +406,8 @@ Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV1Writer(
       auto writer,
       OpenFileWriter(manifest_list_location, std::move(schema), std::move(file_io),
                      adapter->metadata(), "manifest_file"));
-  return std::make_unique<ManifestListWriter>(std::move(writer), std::move(adapter));
+  return std::unique_ptr<ManifestListWriter>(
+      new ManifestListWriter(std::move(writer), std::move(adapter)));
 }
 
 Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV2Writer(
@@ -424,7 +425,8 @@ Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV2Writer(
       OpenFileWriter(manifest_list_location, std::move(schema), std::move(file_io),
                      adapter->metadata(), "manifest_file"));
 
-  return std::make_unique<ManifestListWriter>(std::move(writer), std::move(adapter));
+  return std::unique_ptr<ManifestListWriter>(
+      new ManifestListWriter(std::move(writer), std::move(adapter)));
 }
 
 Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV3Writer(
@@ -441,7 +443,8 @@ Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeV3Writer(
       auto writer,
       OpenFileWriter(manifest_list_location, std::move(schema), std::move(file_io),
                      adapter->metadata(), "manifest_file"));
-  return std::make_unique<ManifestListWriter>(std::move(writer), std::move(adapter));
+  return std::unique_ptr<ManifestListWriter>(
+      new ManifestListWriter(std::move(writer), std::move(adapter)));
 }
 
 }  // namespace iceberg

src/iceberg/manifest/manifest_writer.h

@@ -37,10 +37,6 @@ 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,
-                 std::string_view manifest_location, std::optional<int64_t> first_row_id);
-
   ~ManifestWriter();
 
   /// \brief Write the entry that all its fields are populated correctly.
@@ -159,11 +155,17 @@ class ICEBERG_EXPORT ManifestWriter {
       std::shared_ptr<Schema> current_schema, ManifestContent content);
 
  private:
+  // Private constructor for internal use only, use the static Make*Writer methods
+  // instead.
+  ManifestWriter(std::unique_ptr<Writer> writer,
+                 std::unique_ptr<class ManifestEntryAdapter> adapter,
+                 std::string_view manifest_location, std::optional<int64_t> first_row_id);
+
   Status CheckDataFile(const DataFile& file) const;
 
   static constexpr int64_t kBatchSize = 1024;
   std::unique_ptr<Writer> writer_;
-  std::unique_ptr<ManifestEntryAdapter> adapter_;
+  std::unique_ptr<class ManifestEntryAdapter> adapter_;
   bool closed_{false};
   std::string manifest_location_;
   std::optional<int64_t> first_row_id_;
@@ -181,9 +183,6 @@ class ICEBERG_EXPORT ManifestWriter {
 /// \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);
-
   ~ManifestListWriter();
 
   /// \brief Write manifest file to manifest list file.
@@ -238,9 +237,14 @@ class ICEBERG_EXPORT ManifestListWriter {
       std::string_view manifest_list_location, std::shared_ptr<FileIO> file_io);
 
  private:
+  // Private constructor for internal use only, use the static Make*Writer methods
+  // instead.
+  ManifestListWriter(std::unique_ptr<Writer> writer,
+                     std::unique_ptr<class ManifestFileAdapter> adapter);
+
   static constexpr int64_t kBatchSize = 1024;
   std::unique_ptr<Writer> writer_;
-  std::unique_ptr<ManifestFileAdapter> adapter_;
+  std::unique_ptr<class ManifestFileAdapter> adapter_;
 };
 
 }  // namespace iceberg

src/iceberg/type_fwd.h

@@ -141,8 +141,6 @@ class ManifestListReader;
 class ManifestListWriter;
 class ManifestReader;
 class ManifestWriter;
-class ManifestEntryAdapter;
-class ManifestFileAdapter;
 
 struct ReaderOptions;
 struct WriterOptions;