use ManifestReader::MakeReader instead of CreateManifestReader

Author: xiao.dong

src/iceberg/manifest_reader.cc

@@ -28,40 +28,31 @@
 namespace iceberg {
 
 Result<std::unique_ptr<ManifestReader>> ManifestReader::MakeReader(
-    const std::string& manifest_location, std::shared_ptr<FileIO> file_io,
+    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 reader,
-      ReaderFactoryRegistry::Open(
-          FileFormatType::kAvro,
-          {.path = manifest_location, .io = std::move(file_io), .projection = schema}));
+      auto reader, ReaderFactoryRegistry::Open(FileFormatType::kAvro,
+                                               {.path = std::string(manifest_location),
+                                                .io = std::move(file_io),
+                                                .projection = schema}));
   return std::make_unique<ManifestReaderImpl>(std::move(reader), std::move(schema));
 }
 
 Result<std::unique_ptr<ManifestListReader>> ManifestListReader::MakeReader(
-    const std::string& manifest_list_location, std::shared_ptr<FileIO> file_io) {
+    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 reader,
-      ReaderFactoryRegistry::Open(FileFormatType::kAvro, {.path = manifest_list_location,
-                                                          .io = std::move(file_io),
-                                                          .projection = schema}));
+  ICEBERG_ASSIGN_OR_RAISE(auto reader, ReaderFactoryRegistry::Open(
+                                           FileFormatType::kAvro,
+                                           {.path = std::string(manifest_list_location),
+                                            .io = std::move(file_io),
+                                            .projection = schema}));
   return std::make_unique<ManifestListReaderImpl>(std::move(reader), std::move(schema));
 }
 
-Result<std::unique_ptr<ManifestListReader>> CreateManifestListReader(
-    std::string_view file_path) {
-  return NotImplemented("CreateManifestListReader is not implemented yet.");
-}
-
-Result<std::unique_ptr<ManifestReader>> CreateManifestReader(std::string_view file_path) {
-  return NotImplemented("CreateManifestReader is not implemented yet.");
-}
-
 }  // namespace iceberg

src/iceberg/manifest_reader.h

@@ -37,8 +37,12 @@ class ICEBERG_EXPORT ManifestReader {
   virtual ~ManifestReader() = default;
   virtual Result<std::vector<ManifestEntry>> Entries() const = 0;
 
+  /// \brief Creates a reader for a manifest file.
+  /// \param manifest_location Path to the manifest file.
+  /// \param file_io File IO implementation to use.
+  /// \return A Result containing the reader or an error.
   static Result<std::unique_ptr<ManifestReader>> MakeReader(
-      const std::string& manifest_location, std::shared_ptr<FileIO> file_io,
+      std::string_view manifest_location, std::shared_ptr<FileIO> file_io,
       std::shared_ptr<Schema> partition_schema);
 };
 
@@ -48,19 +52,12 @@ class ICEBERG_EXPORT ManifestListReader {
   virtual ~ManifestListReader() = default;
   virtual Result<std::vector<ManifestFile>> Files() const = 0;
 
+  /// \brief Creates a reader for the manifest list.
+  /// \param manifest_list_location Path to the manifest list file.
+  /// \param file_io File IO implementation to use.
+  /// \return A Result containing the reader or an error.
   static Result<std::unique_ptr<ManifestListReader>> MakeReader(
-      const std::string& manifest_list_location, std::shared_ptr<FileIO> file_io);
+      std::string_view manifest_list_location, std::shared_ptr<FileIO> file_io);
 };
 
-/// \brief Creates a reader for the manifest list.
-/// \param file_path Path to the manifest list file.
-/// \return A Result containing the reader or an error.
-Result<std::unique_ptr<ManifestListReader>> CreateManifestListReader(
-    std::string_view file_path);
-
-/// \brief Creates a reader for a manifest file.
-/// \param file_path Path to the manifest file.
-/// \return A Result containing the reader or an error.
-Result<std::unique_ptr<ManifestReader>> CreateManifestReader(std::string_view file_path);
-
 }  // namespace iceberg

src/iceberg/manifest_reader_internal.cc

@@ -218,7 +218,7 @@ Result<std::vector<ManifestFile>> ParseManifestListEntry(ArrowSchema* schema,
     }
   }
   return manifest_files;
-}  // namespace iceberg
+}
 
 Result<std::vector<ManifestEntry>> ManifestReaderImpl::Entries() const { return {}; }
 

src/iceberg/table_scan.cc

@@ -161,14 +161,17 @@ DataTableScan::DataTableScan(TableScanContext context, std::shared_ptr<FileIO> f
     : TableScan(std::move(context), std::move(file_io)) {}
 
 Result<std::vector<std::shared_ptr<FileScanTask>>> DataTableScan::PlanFiles() const {
-  ICEBERG_ASSIGN_OR_RAISE(auto manifest_list_reader,
-                          CreateManifestListReader(context_.snapshot->manifest_list));
+  ICEBERG_ASSIGN_OR_RAISE(
+      auto manifest_list_reader,
+      ManifestListReader::MakeReader(context_.snapshot->manifest_list, file_io_));
   ICEBERG_ASSIGN_OR_RAISE(auto manifest_files, manifest_list_reader->Files());
 
   std::vector<std::shared_ptr<FileScanTask>> tasks;
   for (const auto& manifest_file : manifest_files) {
-    ICEBERG_ASSIGN_OR_RAISE(auto manifest_reader,
-                            CreateManifestReader(manifest_file.manifest_path));
+    ICEBERG_ASSIGN_OR_RAISE(
+        auto manifest_reader,
+        ManifestReader::MakeReader(manifest_file.manifest_path, file_io_,
+                                   /* TODO(xiao.dong) partition schema*/ nullptr));
     ICEBERG_ASSIGN_OR_RAISE(auto manifests, manifest_reader->Entries());
 
     // TODO(gty404): filter manifests using partition spec and filter expression