SteNicholas commented on code in PR #274:
URL: https://github.com/apache/paimon-cpp/pull/274#discussion_r4036417347
##########
src/paimon/core/table/source/table_scan.cpp:
##########
@@ -97,6 +99,15 @@ class TableScanImpl {
auto manifest_file_format = core_options.GetManifestFormat();
std::string branch =
BranchManager::NormalizeBranch(core_options.GetBranch());
auto snapshot_manager = std::make_shared<SnapshotManager>(fs,
context->GetPath(), branch);
+ if (VersionManagedCatalog* versioned =
AsVersionManaged(context->GetCatalog())) {
+ std::shared_ptr<Catalog> catalog = context->GetCatalog();
+ Identifier identifier = context->GetIdentifier().value();
+ // Keep the catalog alive while the scan uses its snapshot loader.
+ snapshot_manager->SetSnapshotLoader(
+ [catalog, versioned, identifier]() ->
Result<std::optional<Snapshot>> {
+ return versioned->LoadSnapshot(identifier);
+ });
Review Comment:
**Keep the catalog snapshot body when planning the initial scan**.
Installing this loader does not make the default scan work when the latest
snapshot exists only in the catalog. `FullStartingScanner::Scan()` calls
`LatestSnapshotId()` (which now uses this loader), then calls
`SnapshotManager::LoadSnapshot(id)`, which unconditionally reads
`<table>/snapshot/snapshot-<id>` from the filesystem. Consequently both a
default batch scan and the initial full streaming scan fail with `NotExist`
even though the catalog returned a valid snapshot. The new
`TestCatalogScanReadsSnapshotFromCatalog` leaves the fixture's snapshot file in
place and therefore masks this failure. Please carry the returned `Snapshot`
into the initial plan, and cover a catalog-held snapshot with no corresponding
snapshot file.
--
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]
