Skip to content

Commit f3cd338

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

File tree

3 files changed

+47
-38
lines changed

3 files changed

+47
-38
lines changed

src/iceberg/manifest/manifest_list.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,9 @@ struct ICEBERG_EXPORT ManifestFile {
230230
kFirstRowIdFieldId, "first_row_id", int64(),
231231
"Starting row ID to assign to new rows in ADDED data files");
232232

233-
bool operator==(const ManifestFile& other) const = default;
233+
bool operator==(const ManifestFile& other) const {
234+
return manifest_path == other.manifest_path;
235+
}
234236

235237
static const std::shared_ptr<StructType>& Type();
236238
};
@@ -275,7 +277,7 @@ ICEBERG_EXPORT inline constexpr Result<ManifestContent> ManifestContentFromStrin
275277

276278
namespace std {
277279
template <>
278-
struct std::hash<iceberg::ManifestFile> {
280+
struct hash<iceberg::ManifestFile> {
279281
size_t operator()(const iceberg::ManifestFile& manifest_file) const {
280282
return std::hash<std::string>{}(manifest_file.manifest_path);
281283
}

src/iceberg/table_scan.cc

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,30 @@ 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+
if (scan.metadata()->current_snapshot_id == kInvalidSnapshotId) {
651+
return std::vector<std::shared_ptr<ScanTaskType>>{};
652+
}
653+
}
654+
655+
ICEBERG_ASSIGN_OR_RAISE(
656+
int64_t to_snapshot_id_inclusive,
657+
internal::ToSnapshotIdInclusive(scan.context(), *scan.metadata()));
658+
ICEBERG_ASSIGN_OR_RAISE(
659+
std::optional<int64_t> from_snapshot_id_exclusive,
660+
internal::FromSnapshotIdExclusive(scan.context(), *scan.metadata(),
661+
to_snapshot_id_inclusive));
662+
663+
return scan.PlanFiles(from_snapshot_id_exclusive, to_snapshot_id_inclusive);
664+
}
665+
642666
// IncrementalAppendScan implementation
643667

644668
Result<std::unique_ptr<IncrementalAppendScan>> IncrementalAppendScan::Make(
@@ -651,6 +675,11 @@ Result<std::unique_ptr<IncrementalAppendScan>> IncrementalAppendScan::Make(
651675
std::move(metadata), std::move(schema), std::move(io), std::move(context)));
652676
}
653677

678+
Result<std::vector<std::shared_ptr<FileScanTask>>> IncrementalAppendScan::PlanFiles()
679+
const {
680+
return ResolvePlanFiles<FileScanTask>(*this);
681+
}
682+
654683
Result<std::vector<std::shared_ptr<FileScanTask>>> IncrementalAppendScan::PlanFiles(
655684
std::optional<int64_t> from_snapshot_id_exclusive,
656685
int64_t to_snapshot_id_inclusive) const {
@@ -725,35 +754,15 @@ Result<std::unique_ptr<IncrementalChangelogScan>> IncrementalChangelogScan::Make
725754
return NotImplemented("IncrementalChangelogScan is not implemented");
726755
}
727756

757+
Result<std::vector<std::shared_ptr<ChangelogScanTask>>>
758+
IncrementalChangelogScan::PlanFiles() const {
759+
return ResolvePlanFiles<ChangelogScanTask>(*this);
760+
}
761+
728762
Result<std::vector<std::shared_ptr<ChangelogScanTask>>>
729763
IncrementalChangelogScan::PlanFiles(std::optional<int64_t> from_snapshot_id_exclusive,
730764
int64_t to_snapshot_id_inclusive) const {
731765
return NotImplemented("IncrementalChangelogScan::PlanFiles is not implemented");
732766
}
733767

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-
759768
} // 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)