From be8c5f5d4be82d0f7ff61632bb2c7289060189b1 Mon Sep 17 00:00:00 2001
From: Manu Zhang <owenzhang1990@gmail.com>
Date: Thu, 3 Sep 2026 13:04:03 +0800
Subject: [PATCH 1/6] feat: add fallible iterator utility

---
 src/iceberg/test/CMakeLists.txt   |   1 +
 src/iceberg/test/iterator_test.cc | 117 ++++++++++++++++++++++++++++++
 src/iceberg/test/meson.build      |   1 +
 src/iceberg/type_fwd.h            |   2 +
 src/iceberg/util/iterator.h       |  93 ++++++++++++++++++++++++
 src/iceberg/util/meson.build      |   1 +
 6 files changed, 215 insertions(+)
 create mode 100644 src/iceberg/test/iterator_test.cc
 create mode 100644 src/iceberg/util/iterator.h

diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt
index 1181a722e..76f5452f8 100644
--- a/src/iceberg/test/CMakeLists.txt
+++ b/src/iceberg/test/CMakeLists.txt
@@ -133,6 +133,7 @@ add_iceberg_test(util_test
                  endian_test.cc
                  file_io_test.cc
                  formatter_test.cc
+                 iterator_test.cc
                  lazy_test.cc
                  location_util_test.cc
                  math_util_internal_test.cc
diff --git a/src/iceberg/test/iterator_test.cc b/src/iceberg/test/iterator_test.cc
new file mode 100644
index 000000000..1aa97247b
--- /dev/null
+++ b/src/iceberg/test/iterator_test.cc
@@ -0,0 +1,117 @@
+/*
+ * 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/util/iterator.h"
+
+#include <memory>
+#include <optional>
+#include <type_traits>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "iceberg/test/matchers.h"
+
+namespace iceberg {
+namespace {
+
+class CopyOnly {
+ public:
+  explicit CopyOnly(int value) : value_(value) {}
+
+  CopyOnly(const CopyOnly&) = default;
+  CopyOnly& operator=(const CopyOnly&) = default;
+  CopyOnly(CopyOnly&&) = delete;
+  CopyOnly& operator=(CopyOnly&&) = delete;
+
+  int value() const { return value_; }
+
+ private:
+  int value_;
+};
+
+static_assert(std::is_copy_constructible_v<CopyOnly>);
+static_assert(!std::is_move_constructible_v<CopyOnly>);
+
+class CopyOnlyIterator final : public Iterator<CopyOnly> {
+ public:
+  Result<std::optional<CopyOnly>> Next() override {
+    if (next_ == 3) {
+      return Result<std::optional<CopyOnly>>(std::in_place, std::nullopt);
+    }
+    return Result<std::optional<CopyOnly>>(std::in_place, std::in_place, next_++);
+  }
+
+ private:
+  int next_ = 0;
+};
+
+class MoveOnlyIterator final : public Iterator<std::unique_ptr<int>> {
+ public:
+  Result<std::optional<std::unique_ptr<int>>> Next() override {
+    if (next_ == 3) {
+      return Result<std::optional<std::unique_ptr<int>>>(std::in_place,
+                                                         std::nullopt);
+    }
+    return Result<std::optional<std::unique_ptr<int>>>(
+        std::in_place, std::in_place, std::make_unique<int>(next_++));
+  }
+
+ private:
+  int next_ = 0;
+};
+
+class FailingIterator final : public Iterator<int> {
+ public:
+  Result<std::optional<int>> Next() override { return Invalid("iteration failed"); }
+};
+
+TEST(IteratorTest, ToVectorSupportsCopyOnlyValues) {
+  CopyOnlyIterator iterator;
+
+  ICEBERG_UNWRAP_OR_FAIL(auto values, iterator.ToVector());
+
+  ASSERT_EQ(values.size(), 3);
+  EXPECT_EQ(values[0].value(), 0);
+  EXPECT_EQ(values[1].value(), 1);
+  EXPECT_EQ(values[2].value(), 2);
+}
+
+TEST(IteratorTest, ToVectorSupportsMoveOnlyValues) {
+  MoveOnlyIterator iterator;
+
+  ICEBERG_UNWRAP_OR_FAIL(auto values, iterator.ToVector());
+
+  ASSERT_EQ(values.size(), 3);
+  EXPECT_EQ(*values[0], 0);
+  EXPECT_EQ(*values[1], 1);
+  EXPECT_EQ(*values[2], 2);
+}
+
+TEST(IteratorTest, ToVectorPropagatesErrors) {
+  FailingIterator iterator;
+
+  auto result = iterator.ToVector();
+
+  EXPECT_THAT(result, IsError(ErrorKind::kInvalid));
+  EXPECT_THAT(result, HasErrorMessage("iteration failed"));
+}
+
+}  // namespace
+}  // namespace iceberg
diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build
index 6dde45ee9..977b8041f 100644
--- a/src/iceberg/test/meson.build
+++ b/src/iceberg/test/meson.build
@@ -108,6 +108,7 @@ iceberg_tests = {
             'executor_util_test.cc',
             'file_io_test.cc',
             'formatter_test.cc',
+            'iterator_test.cc',
             'lazy_test.cc',
             'location_util_test.cc',
             'math_util_internal_test.cc',
diff --git a/src/iceberg/type_fwd.h b/src/iceberg/type_fwd.h
index 0d2b17fae..0b19adaf5 100644
--- a/src/iceberg/type_fwd.h
+++ b/src/iceberg/type_fwd.h
@@ -229,6 +229,8 @@ struct SessionContext;
 
 /// \brief Task execution.
 class Executor;
+template <typename T>
+class Iterator;
 
 /// \brief Metrics reporting.
 class MetricsReporter;
diff --git a/src/iceberg/util/iterator.h b/src/iceberg/util/iterator.h
new file mode 100644
index 000000000..f214ba0f3
--- /dev/null
+++ b/src/iceberg/util/iterator.h
@@ -0,0 +1,93 @@
+/*
+ * 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/util/iterator.h
+/// \brief Pull-based iterator interface for fallible, lazily produced values.
+
+#include <deque>
+#include <optional>
+#include <type_traits>
+#include <utility>
+#include <vector>
+
+#include "iceberg/result.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+/// \brief A pull-based iterator whose reads may fail.
+///
+/// Iterator implementations own any resources needed to produce values. Destroying an
+/// iterator releases those resources, including when iteration stops before reaching the
+/// end. Iterators are not thread-safe unless an implementation explicitly says otherwise.
+///
+/// \tparam T Value returned by the iterator.
+template <typename T>
+class Iterator {
+ public:
+  virtual ~Iterator() = default;
+
+  Iterator() = default;
+  Iterator(const Iterator&) = delete;
+  Iterator& operator=(const Iterator&) = delete;
+
+  /// \brief Return the next value, or std::nullopt when the iterator is exhausted.
+  virtual Result<std::optional<T>> Next() = 0;
+
+  /// \brief Consume the remaining values into a vector.
+  Result<std::vector<T>> ToVector() {
+    if constexpr (!std::is_move_constructible_v<T>) {
+      static_assert(std::is_copy_constructible_v<T>,
+                    "Iterator::ToVector requires T to be move- or copy-constructible");
+
+      // A vector cannot grow portably when T has an explicitly deleted move
+      // constructor. Stage copy-only values in a deque, then use vector's
+      // forward-range constructor to allocate the final storage once.
+      std::deque<T> values;
+      while (true) {
+        auto result = Next();
+        if (!result.has_value()) {
+          return std::unexpected(std::move(result.error()));
+        }
+        auto& value = result.value();
+        if (!value.has_value()) {
+          return std::vector<T>(values.cbegin(), values.cend());
+        }
+        values.push_back(value.value());
+      }
+    } else {
+      std::vector<T> values;
+      while (true) {
+        auto result = Next();
+        if (!result.has_value()) {
+          return std::unexpected(std::move(result.error()));
+        }
+        auto& value = result.value();
+        if (!value.has_value()) {
+          return values;
+        }
+        values.push_back(std::move_if_noexcept(value.value()));
+      }
+    }
+  }
+};
+
+}  // namespace iceberg
diff --git a/src/iceberg/util/meson.build b/src/iceberg/util/meson.build
index f9436e7ed..831cc5888 100644
--- a/src/iceberg/util/meson.build
+++ b/src/iceberg/util/meson.build
@@ -32,6 +32,7 @@ install_headers(
         'formatter.h',
         'functional.h',
         'int128.h',
+        'iterator.h',
         'lazy.h',
         'location_util.h',
         'macros.h',

From dd09c4672784a4653418fbada9621ac25f6d7393 Mon Sep 17 00:00:00 2001
From: Manu Zhang <owenzhang1990@gmail.com>
Date: Thu, 3 Sep 2026 14:45:41 +0800
Subject: [PATCH 2/6] fix: address iterator review feedback

---
 src/iceberg/test/iterator_test.cc | 26 ++++++++++++++++++++------
 src/iceberg/util/iterator.h       |  6 ++----
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/iceberg/test/iterator_test.cc b/src/iceberg/test/iterator_test.cc
index 1aa97247b..1bce53aab 100644
--- a/src/iceberg/test/iterator_test.cc
+++ b/src/iceberg/test/iterator_test.cc
@@ -49,6 +49,7 @@ class CopyOnly {
 static_assert(std::is_copy_constructible_v<CopyOnly>);
 static_assert(!std::is_move_constructible_v<CopyOnly>);
 
+// Exercises ToVector() with values that can be copied but not moved.
 class CopyOnlyIterator final : public Iterator<CopyOnly> {
  public:
   Result<std::optional<CopyOnly>> Next() override {
@@ -62,24 +63,33 @@ class CopyOnlyIterator final : public Iterator<CopyOnly> {
   int next_ = 0;
 };
 
+// Exercises ToVector() with values that can be moved but not copied.
 class MoveOnlyIterator final : public Iterator<std::unique_ptr<int>> {
  public:
   Result<std::optional<std::unique_ptr<int>>> Next() override {
     if (next_ == 3) {
-      return Result<std::optional<std::unique_ptr<int>>>(std::in_place,
-                                                         std::nullopt);
+      return Result<std::optional<std::unique_ptr<int>>>(std::in_place, std::nullopt);
     }
-    return Result<std::optional<std::unique_ptr<int>>>(
-        std::in_place, std::in_place, std::make_unique<int>(next_++));
+    return Result<std::optional<std::unique_ptr<int>>>(std::in_place, std::in_place,
+                                                       std::make_unique<int>(next_++));
   }
 
  private:
   int next_ = 0;
 };
 
+// Exercises ToVector() error propagation after some values have been consumed.
 class FailingIterator final : public Iterator<int> {
  public:
-  Result<std::optional<int>> Next() override { return Invalid("iteration failed"); }
+  Result<std::optional<int>> Next() override {
+    if (next_ < 2) {
+      return Result<std::optional<int>>(std::in_place, std::in_place, next_++);
+    }
+    return Invalid("iteration failed");
+  }
+
+ private:
+  int next_ = 0;
 };
 
 TEST(IteratorTest, ToVectorSupportsCopyOnlyValues) {
@@ -104,9 +114,13 @@ TEST(IteratorTest, ToVectorSupportsMoveOnlyValues) {
   EXPECT_EQ(*values[2], 2);
 }
 
-TEST(IteratorTest, ToVectorPropagatesErrors) {
+TEST(IteratorTest, ToVectorPropagatesErrorsAfterPartialConsumption) {
   FailingIterator iterator;
 
+  ICEBERG_UNWRAP_OR_FAIL(auto first, iterator.Next());
+  ASSERT_TRUE(first.has_value());
+  EXPECT_EQ(first.value(), 0);
+
   auto result = iterator.ToVector();
 
   EXPECT_THAT(result, IsError(ErrorKind::kInvalid));
diff --git a/src/iceberg/util/iterator.h b/src/iceberg/util/iterator.h
index f214ba0f3..9b274f726 100644
--- a/src/iceberg/util/iterator.h
+++ b/src/iceberg/util/iterator.h
@@ -29,7 +29,6 @@
 #include <vector>
 
 #include "iceberg/result.h"
-#include "iceberg/util/macros.h"
 
 namespace iceberg {
 
@@ -58,9 +57,8 @@ class Iterator {
       static_assert(std::is_copy_constructible_v<T>,
                     "Iterator::ToVector requires T to be move- or copy-constructible");
 
-      // A vector cannot grow portably when T has an explicitly deleted move
-      // constructor. Stage copy-only values in a deque, then use vector's
-      // forward-range constructor to allocate the final storage once.
+      // Stage copy-only values in a deque to avoid copying previously collected
+      // values during growth, then allocate the final vector storage once.
       std::deque<T> values;
       while (true) {
         auto result = Next();

From 5a6ca6fb8bb96c90b8f00a89d29483d13e87fef7 Mon Sep 17 00:00:00 2001
From: Manu Zhang <owenzhang1990@gmail.com>
Date: Fri, 4 Sep 2026 00:17:02 +0800
Subject: [PATCH 3/6] fix: make iterator termination sticky

Cache terminal iterator results so repeated calls do not advance implementations, and explicitly enable move operations.

Co-authored-by: Codex <codex@openai.com>
---
 src/iceberg/test/iterator_test.cc | 55 +++++++++++++++++++++++++++----
 src/iceberg/util/iterator.h       | 35 +++++++++++++++++++-
 2 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/src/iceberg/test/iterator_test.cc b/src/iceberg/test/iterator_test.cc
index 1bce53aab..49b338697 100644
--- a/src/iceberg/test/iterator_test.cc
+++ b/src/iceberg/test/iterator_test.cc
@@ -51,22 +51,25 @@ static_assert(!std::is_move_constructible_v<CopyOnly>);
 
 // Exercises ToVector() with values that can be copied but not moved.
 class CopyOnlyIterator final : public Iterator<CopyOnly> {
- public:
-  Result<std::optional<CopyOnly>> Next() override {
+ private:
+  Result<std::optional<CopyOnly>> NextImpl() override {
     if (next_ == 3) {
       return Result<std::optional<CopyOnly>>(std::in_place, std::nullopt);
     }
     return Result<std::optional<CopyOnly>>(std::in_place, std::in_place, next_++);
   }
 
- private:
   int next_ = 0;
 };
 
 // Exercises ToVector() with values that can be moved but not copied.
 class MoveOnlyIterator final : public Iterator<std::unique_ptr<int>> {
  public:
-  Result<std::optional<std::unique_ptr<int>>> Next() override {
+  int calls() const { return calls_; }
+
+ private:
+  Result<std::optional<std::unique_ptr<int>>> NextImpl() override {
+    ++calls_;
     if (next_ == 3) {
       return Result<std::optional<std::unique_ptr<int>>>(std::in_place, std::nullopt);
     }
@@ -74,24 +77,33 @@ class MoveOnlyIterator final : public Iterator<std::unique_ptr<int>> {
                                                        std::make_unique<int>(next_++));
   }
 
- private:
   int next_ = 0;
+  int calls_ = 0;
 };
 
 // Exercises ToVector() error propagation after some values have been consumed.
 class FailingIterator final : public Iterator<int> {
  public:
-  Result<std::optional<int>> Next() override {
+  int calls() const { return calls_; }
+
+ private:
+  Result<std::optional<int>> NextImpl() override {
+    ++calls_;
     if (next_ < 2) {
       return Result<std::optional<int>>(std::in_place, std::in_place, next_++);
     }
     return Invalid("iteration failed");
   }
 
- private:
   int next_ = 0;
+  int calls_ = 0;
 };
 
+static_assert(std::is_move_constructible_v<CopyOnlyIterator>);
+static_assert(std::is_move_assignable_v<CopyOnlyIterator>);
+static_assert(std::is_move_constructible_v<MoveOnlyIterator>);
+static_assert(std::is_move_assignable_v<MoveOnlyIterator>);
+
 TEST(IteratorTest, ToVectorSupportsCopyOnlyValues) {
   CopyOnlyIterator iterator;
 
@@ -114,6 +126,20 @@ TEST(IteratorTest, ToVectorSupportsMoveOnlyValues) {
   EXPECT_EQ(*values[2], 2);
 }
 
+TEST(IteratorTest, NextRemainsAtEndAfterExhaustion) {
+  MoveOnlyIterator iterator;
+  ICEBERG_UNWRAP_OR_FAIL(auto values, iterator.ToVector());
+  ASSERT_EQ(values.size(), 3);
+  EXPECT_EQ(iterator.calls(), 4);
+
+  for (int i = 0; i < 2; ++i) {
+    auto result = iterator.Next();
+    ASSERT_TRUE(result.has_value());
+    EXPECT_FALSE(result->has_value());
+  }
+  EXPECT_EQ(iterator.calls(), 4);
+}
+
 TEST(IteratorTest, ToVectorPropagatesErrorsAfterPartialConsumption) {
   FailingIterator iterator;
 
@@ -127,5 +153,20 @@ TEST(IteratorTest, ToVectorPropagatesErrorsAfterPartialConsumption) {
   EXPECT_THAT(result, HasErrorMessage("iteration failed"));
 }
 
+TEST(IteratorTest, NextRepeatsErrorWithoutAdvancing) {
+  FailingIterator iterator;
+  auto first_error = iterator.ToVector();
+  EXPECT_THAT(first_error, IsError(ErrorKind::kInvalid));
+  EXPECT_THAT(first_error, HasErrorMessage("iteration failed"));
+  EXPECT_EQ(iterator.calls(), 3);
+
+  for (int i = 0; i < 2; ++i) {
+    auto result = iterator.Next();
+    EXPECT_THAT(result, IsError(ErrorKind::kInvalid));
+    EXPECT_THAT(result, HasErrorMessage("iteration failed"));
+  }
+  EXPECT_EQ(iterator.calls(), 3);
+}
+
 }  // namespace
 }  // namespace iceberg
diff --git a/src/iceberg/util/iterator.h b/src/iceberg/util/iterator.h
index 9b274f726..0abaefdbf 100644
--- a/src/iceberg/util/iterator.h
+++ b/src/iceberg/util/iterator.h
@@ -37,6 +37,8 @@ namespace iceberg {
 /// Iterator implementations own any resources needed to produce values. Destroying an
 /// iterator releases those resources, including when iteration stops before reaching the
 /// end. Iterators are not thread-safe unless an implementation explicitly says otherwise.
+/// Once Next() returns an error or std::nullopt, the iterator is terminal. Subsequent
+/// calls return the same terminal result without invoking the implementation again.
 ///
 /// \tparam T Value returned by the iterator.
 template <typename T>
@@ -47,9 +49,29 @@ class Iterator {
   Iterator() = default;
   Iterator(const Iterator&) = delete;
   Iterator& operator=(const Iterator&) = delete;
+  Iterator(Iterator&&) noexcept = default;
+  Iterator& operator=(Iterator&&) noexcept = default;
 
   /// \brief Return the next value, or std::nullopt when the iterator is exhausted.
-  virtual Result<std::optional<T>> Next() = 0;
+  ///
+  /// After this method returns an error or std::nullopt, subsequent calls return the same
+  /// terminal result without invoking NextImpl().
+  virtual Result<std::optional<T>> Next() final {
+    if (error_.has_value()) {
+      return std::unexpected(*error_);
+    }
+    if (finished_) {
+      return std::nullopt;
+    }
+
+    auto result = NextImpl();
+    if (!result.has_value()) {
+      error_ = result.error();
+    } else if (!result.value().has_value()) {
+      finished_ = true;
+    }
+    return result;
+  }
 
   /// \brief Consume the remaining values into a vector.
   Result<std::vector<T>> ToVector() {
@@ -86,6 +108,17 @@ class Iterator {
       }
     }
   }
+
+ protected:
+  /// \brief Produce the next value for Next().
+  ///
+  /// Implementations must return std::nullopt when exhausted. Next() makes the
+  /// terminal state sticky, so implementations are not called after exhaustion or error.
+  virtual Result<std::optional<T>> NextImpl() = 0;
+
+ private:
+  bool finished_ = false;
+  std::optional<Error> error_;
 };
 
 }  // namespace iceberg

From de45a780e8f0c4cc9ba8cb5cf258769e3a3c06a6 Mon Sep 17 00:00:00 2001
From: Manu Zhang <owenzhang1990@gmail.com>
Date: Fri, 4 Sep 2026 10:08:45 +0800
Subject: [PATCH 4/6] fix: simplify iterator customization

---
 src/iceberg/util/iterator.h | 46 ++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/src/iceberg/util/iterator.h b/src/iceberg/util/iterator.h
index 0abaefdbf..4900cf215 100644
--- a/src/iceberg/util/iterator.h
+++ b/src/iceberg/util/iterator.h
@@ -56,7 +56,7 @@ class Iterator {
   ///
   /// After this method returns an error or std::nullopt, subsequent calls return the same
   /// terminal result without invoking NextImpl().
-  virtual Result<std::optional<T>> Next() final {
+  Result<std::optional<T>> Next() {
     if (error_.has_value()) {
       return std::unexpected(*error_);
     }
@@ -75,13 +75,8 @@ class Iterator {
 
   /// \brief Consume the remaining values into a vector.
   Result<std::vector<T>> ToVector() {
-    if constexpr (!std::is_move_constructible_v<T>) {
-      static_assert(std::is_copy_constructible_v<T>,
-                    "Iterator::ToVector requires T to be move- or copy-constructible");
-
-      // Stage copy-only values in a deque to avoid copying previously collected
-      // values during growth, then allocate the final vector storage once.
-      std::deque<T> values;
+    auto collect = [this](auto& values, auto append,
+                          auto finish) -> Result<std::vector<T>> {
       while (true) {
         auto result = Next();
         if (!result.has_value()) {
@@ -89,23 +84,32 @@ class Iterator {
         }
         auto& value = result.value();
         if (!value.has_value()) {
-          return std::vector<T>(values.cbegin(), values.cend());
+          return finish(values);
         }
-        values.push_back(value.value());
+        append(values, value.value());
       }
+    };
+
+    if constexpr (!std::is_move_constructible_v<T>) {
+      static_assert(std::is_copy_constructible_v<T>,
+                    "Iterator::ToVector requires T to be move- or copy-constructible");
+
+      // Stage copy-only values in a deque to avoid copying previously collected
+      // values during growth, then allocate the final vector storage once.
+      std::deque<T> values;
+      return collect(
+          values, [](auto& destination, const T& value) { destination.push_back(value); },
+          [](const auto& source) {
+            return std::vector<T>(source.cbegin(), source.cend());
+          });
     } else {
       std::vector<T> values;
-      while (true) {
-        auto result = Next();
-        if (!result.has_value()) {
-          return std::unexpected(std::move(result.error()));
-        }
-        auto& value = result.value();
-        if (!value.has_value()) {
-          return values;
-        }
-        values.push_back(std::move_if_noexcept(value.value()));
-      }
+      return collect(
+          values,
+          [](auto& destination, T& value) {
+            destination.push_back(std::move_if_noexcept(value));
+          },
+          [](auto& source) { return std::move(source); });
     }
   }
 

From dc47277b5bc99ce172842cf681cea0f505eb48db Mon Sep 17 00:00:00 2001
From: Manu Zhang <owenzhang1990@gmail.com>
Date: Fri, 4 Sep 2026 10:26:24 +0800
Subject: [PATCH 5/6] perf: collect iterator values directly

---
 src/iceberg/util/iterator.h | 48 ++++++++++---------------------------
 1 file changed, 13 insertions(+), 35 deletions(-)

diff --git a/src/iceberg/util/iterator.h b/src/iceberg/util/iterator.h
index 4900cf215..7b95145b1 100644
--- a/src/iceberg/util/iterator.h
+++ b/src/iceberg/util/iterator.h
@@ -22,7 +22,6 @@
 /// \file iceberg/util/iterator.h
 /// \brief Pull-based iterator interface for fallible, lazily produced values.
 
-#include <deque>
 #include <optional>
 #include <type_traits>
 #include <utility>
@@ -75,41 +74,20 @@ class Iterator {
 
   /// \brief Consume the remaining values into a vector.
   Result<std::vector<T>> ToVector() {
-    auto collect = [this](auto& values, auto append,
-                          auto finish) -> Result<std::vector<T>> {
-      while (true) {
-        auto result = Next();
-        if (!result.has_value()) {
-          return std::unexpected(std::move(result.error()));
-        }
-        auto& value = result.value();
-        if (!value.has_value()) {
-          return finish(values);
-        }
-        append(values, value.value());
-      }
-    };
-
-    if constexpr (!std::is_move_constructible_v<T>) {
-      static_assert(std::is_copy_constructible_v<T>,
-                    "Iterator::ToVector requires T to be move- or copy-constructible");
+    static_assert(std::is_move_constructible_v<T> || std::is_copy_constructible_v<T>,
+                  "Iterator::ToVector requires T to be move- or copy-constructible");
 
-      // Stage copy-only values in a deque to avoid copying previously collected
-      // values during growth, then allocate the final vector storage once.
-      std::deque<T> values;
-      return collect(
-          values, [](auto& destination, const T& value) { destination.push_back(value); },
-          [](const auto& source) {
-            return std::vector<T>(source.cbegin(), source.cend());
-          });
-    } else {
-      std::vector<T> values;
-      return collect(
-          values,
-          [](auto& destination, T& value) {
-            destination.push_back(std::move_if_noexcept(value));
-          },
-          [](auto& source) { return std::move(source); });
+    std::vector<T> values;
+    while (true) {
+      auto result = Next();
+      if (!result.has_value()) {
+        return std::unexpected(std::move(result.error()));
+      }
+      auto& value = result.value();
+      if (!value.has_value()) {
+        return values;
+      }
+      values.push_back(std::move_if_noexcept(value.value()));
     }
   }
 

From cc2fdebc716d4775918d4051b9c25b2113b2d5e8 Mon Sep 17 00:00:00 2001
From: Manu Zhang <owenzhang1990@gmail.com>
Date: Fri, 4 Sep 2026 10:43:07 +0800
Subject: [PATCH 6/6] fix: support copy-only iterator values with libc++

---
 src/iceberg/util/iterator.h | 49 +++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/src/iceberg/util/iterator.h b/src/iceberg/util/iterator.h
index 7b95145b1..7286e1358 100644
--- a/src/iceberg/util/iterator.h
+++ b/src/iceberg/util/iterator.h
@@ -22,6 +22,7 @@
 /// \file iceberg/util/iterator.h
 /// \brief Pull-based iterator interface for fallible, lazily produced values.
 
+#include <deque>
 #include <optional>
 #include <type_traits>
 #include <utility>
@@ -74,20 +75,42 @@ class Iterator {
 
   /// \brief Consume the remaining values into a vector.
   Result<std::vector<T>> ToVector() {
-    static_assert(std::is_move_constructible_v<T> || std::is_copy_constructible_v<T>,
-                  "Iterator::ToVector requires T to be move- or copy-constructible");
-
-    std::vector<T> values;
-    while (true) {
-      auto result = Next();
-      if (!result.has_value()) {
-        return std::unexpected(std::move(result.error()));
-      }
-      auto& value = result.value();
-      if (!value.has_value()) {
-        return values;
+    auto collect = [this](auto& values, auto append,
+                          auto finish) -> Result<std::vector<T>> {
+      while (true) {
+        auto result = Next();
+        if (!result.has_value()) {
+          return std::unexpected(std::move(result.error()));
+        }
+        auto& value = result.value();
+        if (!value.has_value()) {
+          return finish(values);
+        }
+        append(values, value.value());
       }
-      values.push_back(std::move_if_noexcept(value.value()));
+    };
+
+    if constexpr (!std::is_move_constructible_v<T>) {
+      static_assert(std::is_copy_constructible_v<T>,
+                    "Iterator::ToVector requires T to be move- or copy-constructible");
+
+      // Growing a vector requires move-insertable elements in some standard library
+      // implementations. Stage strictly copy-only values in a deque, then copy them
+      // into an exactly sized vector.
+      std::deque<T> values;
+      return collect(
+          values, [](auto& destination, const T& value) { destination.push_back(value); },
+          [](const auto& source) {
+            return std::vector<T>(source.cbegin(), source.cend());
+          });
+    } else {
+      std::vector<T> values;
+      return collect(
+          values,
+          [](auto& destination, T& value) {
+            destination.push_back(std::move_if_noexcept(value));
+          },
+          [](auto& source) { return std::move(source); });
     }
   }