manuzhang opened a new pull request, #873: URL: https://github.com/apache/iceberg-cpp/pull/873
Scan planning currently materializes manifest entries and file scan tasks before returning. This change adds fallible, single-pass streams so callers can consume data-manifest entries and scan tasks incrementally or stop early. Builds on the generic pull utility introduced in #905, renaming it to `Stream<T>` consistently with the scan APIs. The API changes below are intentional; this PR does not preserve source compatibility through adapters or aliases. ## What changed - Add `DataTableScan::PlanFilesStream()` and consuming `ManifestGroup::PlanFilesStream()` APIs. Their eager `PlanFiles()` methods collect the stream into a vector. - Make `ManifestReader::EntriesStream()` and `LiveEntriesStream()` the implementation extension points. Non-virtual `Entries()` and `LiveEntries()` collect these streams for eager callers. - Separate stream interfaces from ownership: `ManifestEntryStream = Stream<ManifestEntry>` and `FileScanTaskStream = Stream<std::shared_ptr<FileScanTask>>`, with corresponding `*Ptr` aliases for `std::unique_ptr`. - `Next()` returns `Result<std::optional<T>>`: an error indicates failure, an empty optional indicates exhaustion, and a value is the next item. Errors and exhaustion are terminal; `ToVector()` collects the remaining items. - Preserve scan metrics for completed and partially consumed streams. Keep partition values for delete matching and residual evaluation even with a narrow manifest projection; drop unrequested statistics only after equality-delete matching. ## Ownership and memory behavior Streams own their reader/planning resources and may outlive the `ManifestReader`, `ManifestGroup`, or `DataTableScan` that created them. A configured executor is borrowed and must remain alive until the planning stream is destroyed. Data-manifest entries are read in bounded batches. Serial planning opens one manifest at a time; executor-backed planning opens at most 32 matching manifest streams per batch and consumes their entries incrementally. Destroying a partially consumed stream releases its resources. This does not make every part of planning lazy: snapshot/manifest-list metadata and the delete-file index remain materialized, and delete manifests are read eagerly before the planning stream is returned. ## Breaking changes and migration ### Generic stream implementations Replace `#include "iceberg/util/iterator.h"` with `#include "iceberg/util/stream.h"`, and rename `Iterator<T>` references/base classes to `Stream<T>`. Keep the `NextImpl()` override and fallible return type. The old header/type and compatibility alias are not retained. ### ManifestGroup callers `ManifestGroup::PlanFiles()` is now rvalue-qualified and consumes its group, as does `PlanFilesStream()`. For a group returned by `ManifestGroup::Make()`: ```cpp // Before: auto tasks = group->PlanFiles(); // After, eager consumption: auto tasks = std::move(*group).PlanFiles(); // Or, incremental consumption (instead of the eager call): auto tasks_stream = std::move(*group).PlanFilesStream(); ``` Do not reuse the consumed group; construct and configure a new one to plan again. For an object rather than a pointer, use `std::move(group)`. `DataTableScan::PlanFiles()` keeps its existing caller syntax and eager result type. ### Custom ManifestReader implementations Replace overrides of `Entries()` and `LiveEntries()` with these required overrides: ```cpp Result<ManifestEntryStreamPtr> EntriesStream() override; Result<ManifestEntryStreamPtr> LiveEntriesStream() override; ``` Implement the entry-producing logic in self-contained streams derived from `ManifestEntryStream`, with `NextImpl()` returning `Result<std::optional<ManifestEntry>>`. Transfer or share all resources needed for consumption; returned streams must remain valid after reader destruction. `LiveEntriesStream()` must exclude deleted entries. Existing eager callers can continue calling `Entries()` and `LiveEntries()`, which now collect these streams. There is no eager-to-stream fallback or separate streaming-capability mixin. ## Validation - Reproduced the V2/V3 equality-delete CI failures locally before the fix, and added coverage for the same projection issue with position deletes. Both regressions pass after the fix, including serial/executor-backed planning and eager/streaming consumption for equality deletes. - Built `manifest_test`, `scan_test`, and `util_test` with CMake on macOS (Debug, static bundle; REST/Hive/SQL catalog disabled). - `ctest --test-dir build --output-on-failure -R '^(manifest_test|scan_test|util_test)$'`: all three suites passed. - `clang-format --dry-run --Werror` for changed C++ files and `git diff --check` passed. -- 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]
