lxy-9602 commented on code in PR #258:
URL: https://github.com/apache/paimon-cpp/pull/258#discussion_r3921532333
##########
src/paimon/core/table/source/data_evolution_batch_scan.cpp:
##########
@@ -44,27 +59,43 @@ DataEvolutionBatchScan::DataEvolutionBatchScan(
executor_(executor) {}
Result<std::shared_ptr<Plan>> DataEvolutionBatchScan::CreatePlan() {
- std::optional<std::vector<Range>> row_ranges;
+ const bool may_use_global_index =
+ global_index_result_ ||
+ (core_options_.GlobalIndexEnabled() &&
batch_scan_->GetNonPartitionPredicate());
+ if (may_use_global_index && UsesUnsupportedTimeTravel(core_options_)) {
+ return Status::NotImplemented("Global index scan does not support time
travel");
+ }
+
+ std::optional<int64_t> global_index_snapshot_id;
std::shared_ptr<GlobalIndexResult> final_global_index_result =
global_index_result_;
if (!final_global_index_result) {
- PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<GlobalIndexResult>
index_result, EvalGlobalIndex());
- if (index_result) {
- final_global_index_result = index_result;
- PAIMON_ASSIGN_OR_RAISE(row_ranges, index_result->ToRanges());
+ PAIMON_ASSIGN_OR_RAISE(std::optional<EvaluatedGlobalIndex>
evaluated_index,
+ EvalGlobalIndex());
+ if (evaluated_index) {
+ final_global_index_result = evaluated_index->result;
+ global_index_snapshot_id = evaluated_index->snapshot_id;
}
- } else {
- PAIMON_ASSIGN_OR_RAISE(row_ranges,
final_global_index_result->ToRanges());
}
- if (!row_ranges) {
+ if (!final_global_index_result) {
return batch_scan_->CreatePlan();
}
- if (row_ranges.value().empty()) {
- return PlanImpl::EmptyPlan();
+ PAIMON_ASSIGN_OR_RAISE(std::vector<Range> row_ranges,
final_global_index_result->ToRanges());
+ if (row_ranges.empty()) {
+ if (global_index_snapshot_id) {
+ return std::make_shared<PlanImpl>(global_index_snapshot_id,
+
std::vector<std::shared_ptr<Split>>());
+ }
+
+ PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<Plan> data_plan,
batch_scan_->CreatePlan());
+ return std::make_shared<PlanImpl>(data_plan->SnapshotId(),
+
std::vector<std::shared_ptr<Split>>());
Review Comment:
Are we sure about this? Do we still need to run a batch scan on an empty
result just to get the snapshot ID? It seems the snapshot should be `null`
here, since we cannot determine the snapshot of an externally provided
`global_index_result`.
--
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]