Skip to content

Commit f7b57f6

Browse files
committed
refactor: simplify lazy file planning
1 parent fd496ae commit f7b57f6

5 files changed

Lines changed: 20 additions & 16 deletions

File tree

src/iceberg/manifest/manifest_group.cc

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ ManifestGroup& ManifestGroup::operator=(ManifestGroup&&) noexcept = default;
134134
class ManifestGroup::FilePlanningIterator final
135135
: public Iterator<std::shared_ptr<FileScanTask>> {
136136
public:
137-
static Result<std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>> Make(
137+
static Result<FileScanTaskIterator> Make(
138138
std::unique_ptr<ManifestGroup> group) {
139139
ICEBERG_RETURN_UNEXPECTED(group->CheckErrors());
140140

@@ -153,7 +153,7 @@ class ManifestGroup::FilePlanningIterator final
153153
group->case_sensitive_));
154154
}
155155

156-
return std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>(
156+
return FileScanTaskIterator(
157157
new FilePlanningIterator(std::move(group), std::move(delete_index),
158158
std::move(data_file_evaluator), drop_stats));
159159
}
@@ -310,11 +310,14 @@ class ManifestGroup::FilePlanningIterator final
310310
ICEBERG_ASSIGN_OR_RAISE(auto evaluator,
311311
GetManifestEvaluator(manifest.partition_spec_id));
312312
ICEBERG_ASSIGN_OR_RAISE(bool should_match, evaluator->Evaluate(manifest));
313-
if (!should_match ||
314-
(group_->ignore_deleted_ && !manifest.has_added_files() &&
315-
!manifest.has_existing_files()) ||
316-
(group_->ignore_existing_ && !manifest.has_added_files() &&
317-
!manifest.has_deleted_files())) {
313+
const bool has_non_deleted_files =
314+
manifest.has_added_files() || manifest.has_existing_files();
315+
const bool has_non_existing_files =
316+
manifest.has_added_files() || manifest.has_deleted_files();
317+
const bool has_only_ignored_files =
318+
(group_->ignore_deleted_ && !has_non_deleted_files) ||
319+
(group_->ignore_existing_ && !has_non_existing_files);
320+
if (!should_match || has_only_ignored_files) {
318321
IncrementSkippedDataManifests();
319322
return false;
320323
}
@@ -553,8 +556,7 @@ Result<std::vector<std::shared_ptr<FileScanTask>>> ManifestGroup::PlanFiles() {
553556
return file_tasks;
554557
}
555558

556-
Result<std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>>
557-
ManifestGroup::PlanFilesIterator() && {
559+
Result<FileScanTaskIterator> ManifestGroup::PlanFilesIterator() && {
558560
auto group = std::make_unique<ManifestGroup>(std::move(*this));
559561
return FilePlanningIterator::Make(std::move(group));
560562
}

src/iceberg/manifest/manifest_group.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class ICEBERG_EXPORT ManifestGroup : public ErrorCollector {
145145
/// manifests in each batch are opened in parallel, while entries are consumed one
146146
/// manifest at a time. Creating the iterator consumes this group's configuration, so
147147
/// this method may only be called on an rvalue.
148-
Result<std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>> PlanFilesIterator() &&;
148+
Result<FileScanTaskIterator> PlanFilesIterator() &&;
149149

150150
/// \brief Get all matching manifest entries.
151151
Result<std::vector<ManifestEntry>> Entries();

src/iceberg/table_scan.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ Result<ScanReport> MakeScanReport(const DataTableScan& scan, const Snapshot& sna
110110
class ReportingFileTaskIterator final : public Iterator<std::shared_ptr<FileScanTask>> {
111111
public:
112112
ReportingFileTaskIterator(
113-
std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>> iterator,
113+
FileScanTaskIterator iterator,
114114
std::shared_ptr<ScanMetrics> scan_metrics,
115115
std::chrono::nanoseconds planning_duration,
116116
std::shared_ptr<MetricsReporter> reporter, ScanReport report)
@@ -147,7 +147,7 @@ class ReportingFileTaskIterator final : public Iterator<std::shared_ptr<FileScan
147147
std::ignore = reporter_->Report(report_);
148148
}
149149

150-
std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>> iterator_;
150+
FileScanTaskIterator iterator_;
151151
std::shared_ptr<ScanMetrics> scan_metrics_;
152152
std::chrono::nanoseconds planning_duration_;
153153
std::shared_ptr<MetricsReporter> reporter_;
@@ -721,8 +721,7 @@ Result<std::vector<std::shared_ptr<FileScanTask>>> DataTableScan::PlanFiles() co
721721
return tasks;
722722
}
723723

724-
Result<std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>>
725-
DataTableScan::PlanFilesIterator() const {
724+
Result<FileScanTaskIterator> DataTableScan::PlanFilesIterator() const {
726725
ICEBERG_ASSIGN_OR_RAISE(auto snapshot, this->snapshot());
727726
if (!snapshot) {
728727
return std::make_unique<EmptyIterator<std::shared_ptr<FileScanTask>>>();

src/iceberg/table_scan.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,8 +468,7 @@ class ICEBERG_EXPORT DataTableScan : public TableScan {
468468
///
469469
/// Unlike PlanFiles(), this method does not materialize all manifest entries and scan
470470
/// tasks. The iterator owns its planning resources and can outlive this scan.
471-
Result<std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>> PlanFilesIterator()
472-
const;
471+
Result<FileScanTaskIterator> PlanFilesIterator() const;
473472

474473
private:
475474
Status ReportScan(const Snapshot& snapshot, const ScanMetrics& scan_metrics) const;

src/iceberg/type_fwd.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
/// you can include this instead of the "full" headers to help reduce compile
2525
/// times.
2626

27+
#include <memory>
28+
2729
namespace iceberg {
2830

2931
/// \brief A data type.
@@ -232,6 +234,8 @@ struct SessionContext;
232234
class Executor;
233235
template <typename T>
234236
class Iterator;
237+
using FileScanTaskIterator =
238+
std::unique_ptr<Iterator<std::shared_ptr<FileScanTask>>>;
235239

236240
/// \brief Metrics reporting.
237241
class MetricsReporter;

0 commit comments

Comments
 (0)