Skip to content

Commit 8e2ad99

Browse files
committed
fix: define IncrementalScan's PlanFiles as pure virtual and no need instantiate in header
1 parent 27d01c9 commit 8e2ad99

File tree

3 files changed

+45
-37
lines changed

3 files changed

+45
-37
lines changed

src/iceberg/manifest/manifest_list.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ ICEBERG_EXPORT inline constexpr Result<ManifestContent> ManifestContentFromStrin
275275

276276
namespace std {
277277
template <>
278-
struct std::hash<iceberg::ManifestFile> {
278+
struct hash<iceberg::ManifestFile> {
279279
size_t operator()(const iceberg::ManifestFile& manifest_file) const {
280280
return std::hash<std::string>{}(manifest_file.manifest_path);
281281
}

src/iceberg/table_scan.cc

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,31 @@ Result<std::vector<std::shared_ptr<FileScanTask>>> DataTableScan::PlanFiles() co
639639
return manifest_group->PlanFiles();
640640
}
641641

642+
// Friend function template for IncrementalScan that implements the shared PlanFiles
643+
// logic. It resolves the from/to snapshot range from the scan context and delegates
644+
// to the two-arg virtual PlanFiles() override in the concrete subclass.
645+
// Defined as a friend to access the protected two-arg PlanFiles().
646+
template <typename ScanTaskType>
647+
Result<std::vector<std::shared_ptr<ScanTaskType>>> ResolvePlanFiles(
648+
const IncrementalScan<ScanTaskType>& scan) {
649+
if (IsScanCurrentLineage(scan.context())) {
650+
ICEBERG_ASSIGN_OR_RAISE(auto current_snapshot, scan.metadata()->Snapshot());
651+
if (current_snapshot == nullptr) {
652+
return std::vector<std::shared_ptr<ScanTaskType>>{};
653+
}
654+
}
655+
656+
ICEBERG_ASSIGN_OR_RAISE(
657+
int64_t to_snapshot_id_inclusive,
658+
internal::ToSnapshotIdInclusive(scan.context(), *scan.metadata()));
659+
ICEBERG_ASSIGN_OR_RAISE(
660+
std::optional<int64_t> from_snapshot_id_exclusive,
661+
internal::FromSnapshotIdExclusive(scan.context(), *scan.metadata(),
662+
to_snapshot_id_inclusive));
663+
664+
return scan.PlanFiles(from_snapshot_id_exclusive, to_snapshot_id_inclusive);
665+
}
666+
642667
// IncrementalAppendScan implementation
643668

644669
Result<std::unique_ptr<IncrementalAppendScan>> IncrementalAppendScan::Make(
@@ -651,6 +676,11 @@ Result<std::unique_ptr<IncrementalAppendScan>> IncrementalAppendScan::Make(
651676
std::move(metadata), std::move(schema), std::move(io), std::move(context)));
652677
}
653678

679+
Result<std::vector<std::shared_ptr<FileScanTask>>> IncrementalAppendScan::PlanFiles()
680+
const {
681+
return ResolvePlanFiles<FileScanTask>(*this);
682+
}
683+
654684
Result<std::vector<std::shared_ptr<FileScanTask>>> IncrementalAppendScan::PlanFiles(
655685
std::optional<int64_t> from_snapshot_id_exclusive,
656686
int64_t to_snapshot_id_inclusive) const {
@@ -725,35 +755,15 @@ Result<std::unique_ptr<IncrementalChangelogScan>> IncrementalChangelogScan::Make
725755
return NotImplemented("IncrementalChangelogScan is not implemented");
726756
}
727757

758+
Result<std::vector<std::shared_ptr<ChangelogScanTask>>>
759+
IncrementalChangelogScan::PlanFiles() const {
760+
return ResolvePlanFiles<ChangelogScanTask>(*this);
761+
}
762+
728763
Result<std::vector<std::shared_ptr<ChangelogScanTask>>>
729764
IncrementalChangelogScan::PlanFiles(std::optional<int64_t> from_snapshot_id_exclusive,
730765
int64_t to_snapshot_id_inclusive) const {
731766
return NotImplemented("IncrementalChangelogScan::PlanFiles is not implemented");
732767
}
733768

734-
// Explicit template implementations for IncrementalScan
735-
// This moves the template implementation from header to source file
736-
template <typename ScanTaskType>
737-
Result<std::vector<std::shared_ptr<ScanTaskType>>>
738-
IncrementalScan<ScanTaskType>::PlanFiles() const {
739-
if (IsScanCurrentLineage(context_)) {
740-
ICEBERG_ASSIGN_OR_RAISE(auto current_snapshot, metadata_->Snapshot());
741-
if (current_snapshot == nullptr) {
742-
return std::vector<std::shared_ptr<ScanTaskType>>{};
743-
}
744-
}
745-
746-
ICEBERG_ASSIGN_OR_RAISE(int64_t to_snapshot_id_inclusive,
747-
internal::ToSnapshotIdInclusive(context_, *metadata_));
748-
ICEBERG_ASSIGN_OR_RAISE(
749-
std::optional<int64_t> from_snapshot_id_exclusive,
750-
internal::FromSnapshotIdExclusive(context_, *metadata_, to_snapshot_id_inclusive));
751-
752-
return PlanFiles(from_snapshot_id_exclusive, to_snapshot_id_inclusive);
753-
}
754-
755-
// Explicitly instantiate the templates
756-
template class IncrementalScan<FileScanTask>;
757-
template class IncrementalScan<ChangelogScanTask>;
758-
759769
} // namespace iceberg

src/iceberg/table_scan.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -356,24 +356,24 @@ class ICEBERG_EXPORT DataTableScan : public TableScan {
356356
/// \brief A base template class for incremental scans that read changes between
357357
/// snapshots, and return scan tasks of the specified type.
358358
template <typename ScanTaskType>
359-
class ICEBERG_EXPORT IncrementalScan : public TableScan {
359+
class IncrementalScan : public TableScan {
360360
public:
361361
~IncrementalScan() override = default;
362362

363-
/// \brief Plans the scan tasks by resolving manifests and data files.
364-
/// \return A Result containing scan tasks or an error.
365-
Result<std::vector<std::shared_ptr<ScanTaskType>>> PlanFiles() const;
363+
virtual Result<std::vector<std::shared_ptr<ScanTaskType>>> PlanFiles() const = 0;
366364

367365
protected:
368366
virtual Result<std::vector<std::shared_ptr<ScanTaskType>>> PlanFiles(
369367
std::optional<int64_t> from_snapshot_id_exclusive,
370368
int64_t to_snapshot_id_inclusive) const = 0;
371369

372370
using TableScan::TableScan;
373-
};
374371

375-
extern template class IncrementalScan<FileScanTask>;
376-
extern template class IncrementalScan<ChangelogScanTask>;
372+
// Allow the free function ResolvePlanFiles to access protected members.
373+
template <typename T>
374+
friend Result<std::vector<std::shared_ptr<T>>> ResolvePlanFiles(
375+
const IncrementalScan<T>& scan);
376+
};
377377

378378
/// \brief A scan that reads data files added between snapshots (incremental appends).
379379
class ICEBERG_EXPORT IncrementalAppendScan : public IncrementalScan<FileScanTask> {
@@ -385,8 +385,7 @@ class ICEBERG_EXPORT IncrementalAppendScan : public IncrementalScan<FileScanTask
385385

386386
~IncrementalAppendScan() override = default;
387387

388-
// Bring the public PlanFiles() from base class into scope
389-
using IncrementalScan<FileScanTask>::PlanFiles;
388+
Result<std::vector<std::shared_ptr<FileScanTask>>> PlanFiles() const override;
390389

391390
protected:
392391
Result<std::vector<std::shared_ptr<FileScanTask>>> PlanFiles(
@@ -407,8 +406,7 @@ class ICEBERG_EXPORT IncrementalChangelogScan
407406

408407
~IncrementalChangelogScan() override = default;
409408

410-
// Bring the public PlanFiles() from base class into scope
411-
using IncrementalScan<ChangelogScanTask>::PlanFiles;
409+
Result<std::vector<std::shared_ptr<ChangelogScanTask>>> PlanFiles() const override;
412410

413411
protected:
414412
Result<std::vector<std::shared_ptr<ChangelogScanTask>>> PlanFiles(

0 commit comments

Comments
 (0)