wangyong9999 commented on code in PR #277:
URL: https://github.com/apache/paimon-cpp/pull/277#discussion_r3916779060
##########
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:
Both branches end with the same GlobalIndexScanImpl::Create + Scan; only
where the snapshot comes from differs. Resolve `snapshot` in the if/else and
keep a single Create/Scan below it, which also removes the `index_scan` /
`index_scan_impl` split.
##########
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:
This wrapper, `Rebind` and `ReadOptimizedBranch` exist only so the view's
path string says `$ro` instead of the base table path. If the view stays bound
to the base table path and the system-table branch of `TableScan::Create`
validates against `system_table_path->table_path`, all three go away, and a
view from a base-table plan can feed a `$ro` scan as well (same snapshot, same
schema).
##########
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:
With a view injected, FullStartingScanner already pins WithSnapshot to that
view's snapshot, and non-latest startup modes are rejected in NewDataTableScan,
so this mismatch cannot occur. Drop the check. The view construction here and
in EmptyPlan() is the same five lines; a small helper would cover both.
--
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]
