wgtmac commented on code in PR #176:
URL: https://github.com/apache/iceberg-cpp/pull/176#discussion_r2278530165


##########
src/iceberg/manifest_writer.cc:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/manifest_writer.h"
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+#include "iceberg/manifest_writer_internal.h"
+#include "iceberg/schema.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t first_row_id,
+    std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
+    std::shared_ptr<Schema> partition_schema) {
+  auto manifest_entry_schema = 
ManifestEntry::TypeFromPartitionType(partition_schema);
+  auto fields_span = manifest_entry_schema->fields();
+  std::vector<SchemaField> fields(fields_span.begin(), fields_span.end());
+  auto schema = std::make_shared<Schema>(fields);
+  ICEBERG_ASSIGN_OR_RAISE(
+      auto writer, WriterFactoryRegistry::Open(FileFormatType::kAvro,
+                                               {.path = 
std::string(manifest_location),
+                                                .schema = schema,
+                                                .io = std::move(file_io)}));
+  switch (format_version) {
+    case 1:
+      return std::make_unique<ManifestWriterV1>(snapshot_id, std::move(writer),
+                                                std::move(schema));
+    case 2:
+      return std::make_unique<ManifestWriterV2>(snapshot_id, std::move(writer),
+                                                std::move(schema));
+    case 3:
+      return std::make_unique<ManifestWriterV3>(snapshot_id, first_row_id,
+                                                std::move(writer), 
std::move(schema));
+
+    default:
+      return InvalidArgument("Unsupported manifest format version: {}", 
format_version);

Review Comment:
   ```suggestion
         return NotSupported("Unsupported manifest format version: {}", 
format_version);
   ```



##########
src/iceberg/v1_metadata.h:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+/// \file iceberg/v1_metadata.h
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+
+namespace iceberg {
+
+/// \brief v1 metadata wrapper.
+///
+/// Wrapper for v1 manifest list and manifest entry.
+class V1MetaData {
+ public:
+  /// \brief v1 manifest file wrapper.
+  struct ManifestFileWrapper : public ManifestFile {
+    ManifestFileWrapper() = default;
+
+    ManifestFile wrap(ManifestFile file, int64_t first_row_id) { return *this; 
}

Review Comment:
   We don't need first_row_id for V1. It is used only by V3.



##########
src/iceberg/manifest_writer.h:
##########
@@ -35,14 +35,24 @@ namespace iceberg {
 class ICEBERG_EXPORT ManifestWriter {
  public:
   virtual ~ManifestWriter() = default;
-  virtual Status WriteManifestEntries(
-      const std::vector<ManifestEntry>& entries) const = 0;
+
+  /// \brief Write manifest entry to file
+  /// \param entry Manifest entry to write.
+  /// \return Status::OK() if all entry was written successfully
+  virtual Status WriteManifestEntry(const ManifestEntry& entry) const = 0;
+
+  /// \brief Close writer and flush to storage.
+  virtual Status Close() = 0;
 
   /// \brief Creates a writer for a manifest file.
+  /// \param format_version Format version of the manifest.
+  /// \param snapshot_id ID of the snapshot.
+  /// \param first_row_id First row ID of the snapshot.
   /// \param manifest_location Path to the manifest file.
   /// \param file_io File IO implementation to use.
   /// \return A Result containing the writer or an error.
   static Result<std::unique_ptr<ManifestWriter>> MakeWriter(

Review Comment:
   ```suggestion
     static Result<std::unique_ptr<ManifestWriter>> Make(
   ```



##########
src/iceberg/manifest_writer.h:
##########
@@ -51,13 +61,27 @@ class ICEBERG_EXPORT ManifestWriter {
 class ICEBERG_EXPORT ManifestListWriter {
  public:
   virtual ~ManifestListWriter() = default;
-  virtual Status WriteManifestFiles(const std::vector<ManifestFile>& files) 
const = 0;
+
+  /// \brief Write manifest file list to manifest list file.
+  /// \param file Manifest file to write.
+  /// \return Status::OK() if all file was written successfully
+  virtual Status WriteManifestFile(const ManifestFile& file) const = 0;

Review Comment:
   We may also change the function name to `Add` and `AddAll` respectively.



##########
src/iceberg/manifest_writer.h:
##########
@@ -35,14 +35,24 @@ namespace iceberg {
 class ICEBERG_EXPORT ManifestWriter {
  public:
   virtual ~ManifestWriter() = default;
-  virtual Status WriteManifestEntries(
-      const std::vector<ManifestEntry>& entries) const = 0;
+
+  /// \brief Write manifest entry to file
+  /// \param entry Manifest entry to write.
+  /// \return Status::OK() if all entry was written successfully
+  virtual Status WriteManifestEntry(const ManifestEntry& entry) const = 0;
+
+  /// \brief Close writer and flush to storage.
+  virtual Status Close() = 0;
 
   /// \brief Creates a writer for a manifest file.
+  /// \param format_version Format version of the manifest.
+  /// \param snapshot_id ID of the snapshot.
+  /// \param first_row_id First row ID of the snapshot.
   /// \param manifest_location Path to the manifest file.
   /// \param file_io File IO implementation to use.
   /// \return A Result containing the writer or an error.
   static Result<std::unique_ptr<ManifestWriter>> MakeWriter(
+      int32_t format_version, int64_t snapshot_id, int64_t first_row_id,

Review Comment:
   Should we make some parameters optional, like first_row_id? Same question 
for ManifestListWriter.



##########
src/iceberg/manifest_writer.cc:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/manifest_writer.h"
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+#include "iceberg/manifest_writer_internal.h"
+#include "iceberg/schema.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t first_row_id,
+    std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
+    std::shared_ptr<Schema> partition_schema) {
+  auto manifest_entry_schema = 
ManifestEntry::TypeFromPartitionType(partition_schema);

Review Comment:
   ```suggestion
     auto manifest_entry_schema = 
ManifestEntry::TypeFromPartitionType(std::move(partition_schema));
   ```



##########
src/iceberg/manifest_writer.h:
##########
@@ -51,13 +61,27 @@ class ICEBERG_EXPORT ManifestWriter {
 class ICEBERG_EXPORT ManifestListWriter {
  public:
   virtual ~ManifestListWriter() = default;
-  virtual Status WriteManifestFiles(const std::vector<ManifestFile>& files) 
const = 0;
+
+  /// \brief Write manifest file list to manifest list file.
+  /// \param file Manifest file to write.
+  /// \return Status::OK() if all file was written successfully
+  virtual Status WriteManifestFile(const ManifestFile& file) const = 0;
+
+  /// \brief Close writer and flush to storage.
+  virtual Status Close() = 0;
 
   /// \brief Creates a writer for the manifest list.
+  /// \param format_version Format version of the manifest list.
+  /// \param snapshot_id ID of the snapshot.
+  /// \param parent_snapshot_id ID of the parent snapshot.
+  /// \param sequence_number Sequence number of the snapshot.
+  /// \param first_row_id First row ID of the snapshot.
   /// \param manifest_list_location Path to the manifest list file.
   /// \param file_io File IO implementation to use.
   /// \return A Result containing the writer or an error.
   static Result<std::unique_ptr<ManifestListWriter>> MakeWriter(

Review Comment:
   ```suggestion
     static Result<std::unique_ptr<ManifestListWriter>> Make(
   ```



##########
src/iceberg/manifest_writer.cc:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/manifest_writer.h"
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+#include "iceberg/manifest_writer_internal.h"
+#include "iceberg/schema.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t first_row_id,
+    std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
+    std::shared_ptr<Schema> partition_schema) {
+  auto manifest_entry_schema = 
ManifestEntry::TypeFromPartitionType(partition_schema);
+  auto fields_span = manifest_entry_schema->fields();
+  std::vector<SchemaField> fields(fields_span.begin(), fields_span.end());
+  auto schema = std::make_shared<Schema>(fields);
+  ICEBERG_ASSIGN_OR_RAISE(
+      auto writer, WriterFactoryRegistry::Open(FileFormatType::kAvro,
+                                               {.path = 
std::string(manifest_location),
+                                                .schema = schema,
+                                                .io = std::move(file_io)}));
+  switch (format_version) {
+    case 1:
+      return std::make_unique<ManifestWriterV1>(snapshot_id, std::move(writer),
+                                                std::move(schema));
+    case 2:
+      return std::make_unique<ManifestWriterV2>(snapshot_id, std::move(writer),
+                                                std::move(schema));
+    case 3:
+      return std::make_unique<ManifestWriterV3>(snapshot_id, first_row_id,
+                                                std::move(writer), 
std::move(schema));
+
+    default:
+      return InvalidArgument("Unsupported manifest format version: {}", 
format_version);
+  }
+}
+
+Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t parent_snapshot_id,
+    int64_t sequence_number, int64_t first_row_id,
+    std::string_view manifest_list_location, std::shared_ptr<FileIO> file_io) {
+  std::vector<SchemaField> fields(ManifestFile::Type().fields().begin(),
+                                  ManifestFile::Type().fields().end());
+  auto schema = std::make_shared<Schema>(fields);
+  ICEBERG_ASSIGN_OR_RAISE(auto writer, WriterFactoryRegistry::Open(
+                                           FileFormatType::kAvro,
+                                           {.path = 
std::string(manifest_list_location),
+                                            .schema = schema,
+                                            .io = std::move(file_io)}));
+  switch (format_version) {
+    case 1:
+      return std::make_unique<ManifestListWriterV1>(snapshot_id, 
parent_snapshot_id,
+
+                                                    std::move(writer), 
std::move(schema));
+    case 2:
+      return std::make_unique<ManifestListWriterV2>(snapshot_id, 
parent_snapshot_id,
+                                                    sequence_number, 
std::move(writer),
+                                                    std::move(schema));
+    case 3:
+      return std::make_unique<ManifestListWriterV3>(snapshot_id, 
parent_snapshot_id,
+                                                    sequence_number, 
first_row_id,
+                                                    std::move(writer), 
std::move(schema));
+
+    default:
+      return InvalidArgument("Unsupported manifest list format version: {}",

Review Comment:
   ```suggestion
         return NotSupported("Unsupported manifest list format version: {}",
   ```



##########
src/iceberg/manifest_writer.cc:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/manifest_writer.h"
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+#include "iceberg/manifest_writer_internal.h"
+#include "iceberg/schema.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t first_row_id,
+    std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
+    std::shared_ptr<Schema> partition_schema) {
+  auto manifest_entry_schema = 
ManifestEntry::TypeFromPartitionType(partition_schema);
+  auto fields_span = manifest_entry_schema->fields();
+  std::vector<SchemaField> fields(fields_span.begin(), fields_span.end());
+  auto schema = std::make_shared<Schema>(fields);

Review Comment:
   nit: you may use `FromStructType` in the src/iceberg/schema_internal.h



##########
src/iceberg/v1_metadata.h:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+/// \file iceberg/v1_metadata.h
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+
+namespace iceberg {
+
+/// \brief v1 metadata wrapper.
+///
+/// Wrapper for v1 manifest list and manifest entry.
+class V1MetaData {
+ public:
+  /// \brief v1 manifest file wrapper.
+  struct ManifestFileWrapper : public ManifestFile {
+    ManifestFileWrapper() = default;
+
+    ManifestFile wrap(ManifestFile file, int64_t first_row_id) { return *this; 
}

Review Comment:
   ```suggestion
       ManifestFile Wrap(ManifestFile file, int64_t first_row_id) { return 
*this; }
   ```



##########
src/iceberg/manifest_writer_internal.h:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+/// \file iceberg/internal/manifest_writer_internal.h
+/// Writer implementation for manifest list files and manifest files.
+
+#include "iceberg/manifest_writer.h"
+#include "iceberg/v1_metadata.h"
+#include "iceberg/v2_metadata.h"
+#include "iceberg/v3_metadata.h"
+
+namespace iceberg {
+
+/// \brief Write manifest entries to a manifest file.
+class ManifestWriterImpl : public ManifestWriter {
+ public:
+  explicit ManifestWriterImpl(int64_t snapshot_id, std::unique_ptr<Writer> 
writer,
+                              std::shared_ptr<Schema> schema)
+      : schema_(std::move(schema)), writer_(std::move(writer)) {}
+
+ private:
+  std::shared_ptr<Schema> schema_;
+  std::unique_ptr<Writer> writer_;
+};
+
+/// \brief Write v1 manifest entries to a manifest file.
+class ManifestWriterV1 : public ManifestWriterImpl {
+ public:
+  explicit ManifestWriterV1(int64_t snapshot_id, std::unique_ptr<Writer> 
writer,
+                            std::shared_ptr<Schema> schema)
+      : ManifestWriterImpl(snapshot_id, std::move(writer), std::move(schema)) 
{}
+
+  Status WriteManifestEntry(const ManifestEntry& entry) const override;
+
+  Status Close() override;
+
+ private:
+  V1MetaData::ManifestEntryWrapper wrapper_;
+};
+
+/// \brief Write v2 manifest entries to a manifest file.
+class ManifestWriterV2 : public ManifestWriterImpl {
+ public:
+  explicit ManifestWriterV2(int64_t snapshot_id, std::unique_ptr<Writer> 
writer,
+                            std::shared_ptr<Schema> schema)
+      : ManifestWriterImpl(snapshot_id, std::move(writer), std::move(schema)),
+        wrapper_(snapshot_id) {}
+
+  Status WriteManifestEntry(const ManifestEntry& entry) const override;
+
+  Status Close() override;
+
+ private:
+  V2MetaData::ManifestEntryWrapper wrapper_;
+};
+
+/// \brief Write v3 manifest entries to a manifest file.
+class ManifestWriterV3 : public ManifestWriterImpl {
+ public:
+  explicit ManifestWriterV3(int64_t snapshot_id, int64_t first_row_id,
+                            std::unique_ptr<Writer> writer,
+                            std::shared_ptr<Schema> schema)
+      : ManifestWriterImpl(snapshot_id, std::move(writer), std::move(schema)),
+        wrapper_(snapshot_id) {}
+
+  Status WriteManifestEntry(const ManifestEntry& entry) const override;
+
+  Status Close() override;
+
+ private:
+  V3MetaData::ManifestEntryWrapper wrapper_;
+};
+
+/// \brief Write manifest files to a manifest list file.
+class ManifestListWriterImpl : public ManifestListWriter {

Review Comment:
   The Java-equivalent has `protected abstract ManifestFile 
prepare(ManifestFile manifest)`, we need to add it too.



##########
src/iceberg/manifest_writer.cc:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/manifest_writer.h"
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+#include "iceberg/manifest_writer_internal.h"
+#include "iceberg/schema.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+Result<std::unique_ptr<ManifestWriter>> ManifestWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t first_row_id,
+    std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
+    std::shared_ptr<Schema> partition_schema) {
+  auto manifest_entry_schema = 
ManifestEntry::TypeFromPartitionType(partition_schema);
+  auto fields_span = manifest_entry_schema->fields();
+  std::vector<SchemaField> fields(fields_span.begin(), fields_span.end());
+  auto schema = std::make_shared<Schema>(fields);
+  ICEBERG_ASSIGN_OR_RAISE(
+      auto writer, WriterFactoryRegistry::Open(FileFormatType::kAvro,
+                                               {.path = 
std::string(manifest_location),
+                                                .schema = schema,
+                                                .io = std::move(file_io)}));
+  switch (format_version) {
+    case 1:
+      return std::make_unique<ManifestWriterV1>(snapshot_id, std::move(writer),
+                                                std::move(schema));
+    case 2:
+      return std::make_unique<ManifestWriterV2>(snapshot_id, std::move(writer),
+                                                std::move(schema));
+    case 3:
+      return std::make_unique<ManifestWriterV3>(snapshot_id, first_row_id,
+                                                std::move(writer), 
std::move(schema));
+
+    default:
+      return InvalidArgument("Unsupported manifest format version: {}", 
format_version);
+  }
+}
+
+Result<std::unique_ptr<ManifestListWriter>> ManifestListWriter::MakeWriter(
+    int32_t format_version, int64_t snapshot_id, int64_t parent_snapshot_id,
+    int64_t sequence_number, int64_t first_row_id,
+    std::string_view manifest_list_location, std::shared_ptr<FileIO> file_io) {
+  std::vector<SchemaField> fields(ManifestFile::Type().fields().begin(),
+                                  ManifestFile::Type().fields().end());
+  auto schema = std::make_shared<Schema>(fields);

Review Comment:
   ditto, you may use FromStructType.



##########
src/iceberg/manifest_writer.h:
##########
@@ -51,13 +61,27 @@ class ICEBERG_EXPORT ManifestWriter {
 class ICEBERG_EXPORT ManifestListWriter {
  public:
   virtual ~ManifestListWriter() = default;
-  virtual Status WriteManifestFiles(const std::vector<ManifestFile>& files) 
const = 0;
+
+  /// \brief Write manifest file list to manifest list file.
+  /// \param file Manifest file to write.
+  /// \return Status::OK() if all file was written successfully
+  virtual Status WriteManifestFile(const ManifestFile& file) const = 0;

Review Comment:
   Add an overload to accept `const std::vector<ManifestFile>&`?



##########
src/iceberg/v2_metadata.h:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+/// \file iceberg/v2_metadata.h
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+
+namespace iceberg {
+
+/// \brief v2 metadata wrapper.
+///
+/// Wrapper for v2 manifest list and manifest entry.
+class V2MetaData {
+ public:
+  /// \brief v2 manifest file wrapper.
+  struct ManifestFileWrapper : public ManifestFile {
+    explicit ManifestFileWrapper(int64_t commit_snapshotId, int64_t 
sequence_number) {}
+
+    ManifestFile wrap(ManifestFile file, int64_t first_row_id) { return *this; 
}

Review Comment:
   ditto for first_row_id



##########
src/iceberg/v1_metadata.h:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+/// \file iceberg/v1_metadata.h
+
+#include "iceberg/manifest_entry.h"
+#include "iceberg/manifest_list.h"
+
+namespace iceberg {
+
+/// \brief v1 metadata wrapper.
+///
+/// Wrapper for v1 manifest list and manifest entry.
+class V1MetaData {
+ public:
+  /// \brief v1 manifest file wrapper.
+  struct ManifestFileWrapper : public ManifestFile {
+    ManifestFileWrapper() = default;
+
+    ManifestFile wrap(ManifestFile file, int64_t first_row_id) { return *this; 
}

Review Comment:
   Do we want to make a copy for every ManifestFile?



-- 
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]

Reply via email to