wangyong9999 commented on code in PR #277:
URL: https://github.com/apache/paimon-cpp/pull/277#discussion_r3916891505
##########
src/paimon/core/table/source/data_evolution_batch_scan.cpp:
##########
@@ -145,26 +160,46 @@ Result<std::shared_ptr<GlobalIndexResult>>
DataEvolutionBatchScan::EvalGlobalInd
return std::shared_ptr<GlobalIndexResult>(nullptr);
}
auto partition_filter = batch_scan_->GetPartitionPredicate();
- // TODO(lisizhuo.lsz): support time travel
- std::optional<Snapshot> snapshot;
- const std::shared_ptr<SnapshotManager>& snapshot_manager =
- snapshot_reader_->GetSnapshotManager();
- if (const std::optional<int64_t>& snapshot_id =
core_options_.GetScanSnapshotId()) {
- PAIMON_ASSIGN_OR_RAISE(Snapshot loaded_snapshot,
-
snapshot_manager->LoadSnapshot(snapshot_id.value()));
- snapshot = std::move(loaded_snapshot);
- } else {
- PAIMON_ASSIGN_OR_RAISE(snapshot, snapshot_manager->LatestSnapshot());
+ StartupMode startup_mode = core_options_.GetStartupMode();
+ if (!(startup_mode == StartupMode::LatestFull() || startup_mode ==
StartupMode::Latest())) {
Review Comment:
Agreed. The snapshot is now resolved in the startup-mode branches, followed
by one `GlobalIndexScanImpl::Create` and `Scan` path. The explicit/non-latest
behavior is unchanged.
##########
src/paimon/core/table/system/read_optimized_system_table.cpp:
##########
@@ -26,14 +26,57 @@
#include "arrow/c/bridge.h"
#include "paimon/common/types/data_field.h"
#include "paimon/core/schema/table_schema.h"
+#include "paimon/core/table/source/plan_impl.h"
#include "paimon/core/table/source/read_optimized_scan_options.h"
+#include "paimon/core/table/source/snapshot_read_view_impl.h"
+#include "paimon/core/utils/branch_manager.h"
#include "paimon/defs.h"
#include "paimon/read_context.h"
#include "paimon/scan_context.h"
#include "paimon/table/source/table_read.h"
#include "paimon/table/source/table_scan.h"
namespace paimon {
+namespace {
+
+class ReadOptimizedTableScan final : public TableScan {
Review Comment:
Agreed. The view identity is now the normalized base-table path plus
normalized branch. The system-table entry validates against the parsed base
path and still accepts views only for `$ro`; the wrapper, `Rebind`, and branch
helper are removed. I also added coverage showing that a base-table view feeds
`$ro` without metadata reload, while branch/table isolation remains enforced.
##########
src/paimon/core/table/source/snapshot/snapshot_reader.cpp:
##########
@@ -47,7 +88,16 @@ Result<std::shared_ptr<Plan>> SnapshotReader::Read() const {
PAIMON_ASSIGN_OR_RAISE(
std::vector<std::shared_ptr<Split>> data_splits,
GenerateSplits(snapshot, scan_mode_ != ScanMode::ALL,
split_generator_, std::move(files)));
- return std::make_shared<PlanImpl>(raw_plan->SnapshotId(), data_splits);
+ std::shared_ptr<const SnapshotReadView> read_view = snapshot_read_view_;
+ if (!read_view && table_schema_) {
+ const std::shared_ptr<SnapshotManager>& snapshot_manager =
GetSnapshotManager();
+ read_view = SnapshotReadViewImpl::Create(
+ snapshot_manager->RootPath(), snapshot_manager->Branch(),
snapshot, table_schema_);
+ }
+ if (read_view && read_view->SnapshotId() != raw_plan->SnapshotId()) {
Review Comment:
Agreed. Under the accepted-view contract, `FullStartingScanner` pins the
injected snapshot and non-latest modes are rejected, so the mismatch check was
redundant. It is removed, and a private const helper now shares view
construction between `Read()` and `EmptyPlan()` without introducing mutable
state.
--
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]
