fix comments

Author: gty404

src/iceberg/manifest_entry.h

@@ -291,7 +291,7 @@ struct ICEBERG_EXPORT ManifestEntry {
   std::optional<int64_t> file_sequence_number;
   /// Field id: 2
   /// File path, partition tuple, metrics, ...
-  DataFile data_file;
+  std::shared_ptr<DataFile> data_file;
 
   inline static const SchemaField kStatus =
       SchemaField::MakeRequired(0, "status", std::make_shared<IntType>());

src/iceberg/manifest_reader.h

@@ -34,19 +34,37 @@ namespace iceberg {
 /// \brief Read manifest entries from a manifest file.
 class ICEBERG_EXPORT ManifestReader {
  public:
+  virtual ~ManifestReader() = default;
   virtual Result<std::span<std::unique_ptr<ManifestEntry>>> Entries() const = 0;
 
  private:
   std::unique_ptr<StructLikeReader> reader_;
 };
 
+/// \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.
+inline Result<std::unique_ptr<ManifestReader>> CreateManifestReader(
+    const std::string& file_path) {
+  return NotImplemented("manifest reader");
+}
+
 /// \brief Read manifest files from a manifest list file.
 class ICEBERG_EXPORT ManifestListReader {
  public:
+  virtual ~ManifestListReader() = default;
   virtual Result<std::span<std::unique_ptr<ManifestFile>>> Files() const = 0;
 
  private:
   std::unique_ptr<StructLikeReader> reader_;
 };
 
+/// \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.
+inline Result<std::unique_ptr<ManifestListReader>> CreateManifestListReader(
+    const std::string& file_path) {
+  return NotImplemented("manifest list reader");
+}
+
 }  // namespace iceberg

src/iceberg/table_scan.cc

@@ -33,8 +33,8 @@ namespace iceberg {
 TableScanBuilder::TableScanBuilder(const Table& table) : table_(table) {}
 
 TableScanBuilder& TableScanBuilder::WithColumnNames(
-    const std::vector<std::string>& column_names) {
-  column_names_ = column_names;
+    std::vector<std::string> column_names) {
+  column_names_ = std::move(column_names);
   return *this;
 }
 
@@ -43,9 +43,8 @@ TableScanBuilder& TableScanBuilder::WithSnapshotId(int64_t snapshot_id) {
   return *this;
 }
 
-TableScanBuilder& TableScanBuilder::WithFilter(
-    const std::shared_ptr<Expression>& filter) {
-  filter_ = filter;
+TableScanBuilder& TableScanBuilder::WithFilter(std::shared_ptr<Expression> filter) {
+  filter_ = std::move(filter);
   return *this;
 }
 
@@ -56,33 +55,37 @@ Result<std::unique_ptr<TableScan>> TableScanBuilder::Build() {
   } else {
     snapshot = table_.current_snapshot();
   }
+  if (snapshot == nullptr) {
+    return InvalidArgument("No snapshot found for table {}", table_.name());
+  }
 
   std::shared_ptr<Schema> schema;
   if (snapshot->schema_id) {
     const auto& schemas = table_.schemas();
     if (auto it = schemas.find(*snapshot->schema_id); it != schemas.end()) {
       schema = it->second;
     } else {
-      return InvalidData("Schema {} in snapshot {} is not found", *snapshot->schema_id,
-                         snapshot->snapshot_id);
+      return InvalidArgument("Schema {} in snapshot {} is not found",
+                             *snapshot->schema_id, snapshot->snapshot_id);
     }
   } else {
     schema = table_.schema();
   }
 
-  std::vector<int32_t> field_ids;
-  field_ids.reserve(column_names_.size());
+  std::vector<SchemaField> project_fields;
+  project_fields.reserve(column_names_.size());
   for (const auto& column_name : column_names_) {
     auto field_opt = schema->GetFieldByName(column_name);
     if (!field_opt) {
       return InvalidArgument("Column {} not found in schema", column_name);
     }
-    field_ids.emplace_back(field_opt.value().get().field_id());
+    project_fields.emplace_back(field_opt.value().get());
   }
 
+  auto projected_schema =
+      std::make_shared<Schema>(std::move(project_fields), schema->schema_id());
   TableScan::ScanContext context{.snapshot = std::move(snapshot),
-                                 .schema = std::move(schema),
-                                 .field_ids = std::move(field_ids),
+                                 .projected_schema = std::move(projected_schema),
                                  .filter = std::move(filter_)};
   return std::make_unique<TableScan>(std::move(context), table_.io());
 }
@@ -103,23 +106,11 @@ Result<std::vector<std::shared_ptr<FileScanTask>>> TableScan::PlanFiles() const
 
     for (const auto& manifest : manifests) {
       const auto& data_file = manifest->data_file;
-      tasks.emplace_back(std::make_shared<FileScanTask>(
-          data_file.file_path, 0, data_file.file_size_in_bytes, data_file.record_count,
-          data_file.content, data_file.file_format, context_.schema, context_.field_ids,
-          context_.filter));
+      tasks.emplace_back(
+          std::make_shared<FileScanTask>(data_file, 0, data_file->file_size_in_bytes));
     }
   }
   return tasks;
 }
 
-Result<std::unique_ptr<ManifestListReader>> TableScan::CreateManifestListReader(
-    const std::string& file_path) const {
-  return NotImplemented("manifest list reader");
-}
-
-Result<std::unique_ptr<ManifestReader>> TableScan::CreateManifestReader(
-    const std::string& file_path) const {
-  return NotImplemented("manifest reader");
-}
-
 }  // namespace iceberg

src/iceberg/table_scan.h

@@ -42,7 +42,7 @@ class ICEBERG_EXPORT TableScanBuilder {
   /// \brief Selects columns to include in the scan.
   /// \param column_names A list of column names. If empty, all columns will be selected.
   /// \return Reference to the builder.
-  TableScanBuilder& WithColumnNames(const std::vector<std::string>& column_names);
+  TableScanBuilder& WithColumnNames(std::vector<std::string> column_names);
 
   /// \brief Applies a filter expression to the scan.
   /// \param filter Filter expression to use.
@@ -65,10 +65,9 @@ class ICEBERG_EXPORT TableScan {
  public:
   /// \brief Scan context holding snapshot and scan-specific metadata.
   struct ScanContext {
-    std::shared_ptr<Snapshot> snapshot;  ///< Snapshot to scan.
-    std::shared_ptr<Schema> schema;      ///< Table schema.
-    std::vector<int32_t> field_ids;      ///< Field IDs of selected columns.
-    std::shared_ptr<Expression> filter;  ///< Filter expression to apply.
+    std::shared_ptr<Snapshot> snapshot;        ///< Snapshot to scan.
+    std::shared_ptr<Schema> projected_schema;  ///< Projected schema.
+    std::shared_ptr<Expression> filter;        ///< Filter expression to apply.
   };
 
   /// \brief Constructs a TableScan with the given context and file I/O.
@@ -83,33 +82,15 @@ class ICEBERG_EXPORT TableScan {
   Result<std::vector<std::shared_ptr<FileScanTask>>> PlanFiles() const;
 
  private:
-  /// \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(
-      const std::string& file_path) const;
-
-  /// \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(
-      const std::string& file_path) const;
-
   ScanContext context_;
   std::shared_ptr<FileIO> file_io_;
 };
 
 /// \brief Represents a task to scan a portion of a data file.
 struct ICEBERG_EXPORT FileScanTask {
-  std::string file_path;                 ///< Path to the data file.
-  uint64_t start;                        ///< Start byte offset.
-  uint64_t length;                       ///< Length in bytes to scan.
-  std::optional<uint64_t> record_count;  ///< Optional number of records.
-  DataFile::Content file_content;        ///< Type of file content.
-  FileFormatType file_format;            ///< Format of the data file.
-  std::shared_ptr<Schema> schema;        ///< Table schema.
-  std::vector<int32_t> field_ids;        ///< Field IDs to project.
-  std::shared_ptr<Expression> filter;    ///< Filter expression to apply.
+  std::shared_ptr<DataFile> data_file;  ///< the data file.
+  uint64_t start;                       ///< Start byte offset.
+  uint64_t length;                      ///< Length in bytes to scan.
 };
 
 }  // namespace iceberg

src/iceberg/type_fwd.h

@@ -131,8 +131,9 @@ class AppendFiles;
 struct DataFile;
 struct ManifestEntry;
 struct ManifestFile;
-class ManifestReader;
 struct ManifestList;
+
+class ManifestReader;
 class ManifestListReader;
 
 }  // namespace iceberg