Skip to content

fix(scan): preserve snapshot id for empty data evolution index plans - #258

Open
wangyong9999 wants to merge 1 commit into
apache:mainfrom
wangyong9999:fix/data-evolution-empty-plan-snapshot-20260828
Open

fix(scan): preserve snapshot id for empty data evolution index plans#258
wangyong9999 wants to merge 1 commit into
apache:mainfrom
wangyong9999:fix/data-evolution-empty-plan-snapshot-20260828

Conversation

@wangyong9999

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: none.

A global-index miss on a non-empty DataEvolution table currently returns PlanImpl::EmptyPlan(). The query result is correctly empty, but the plan loses the snapshot used by the index scan; a null snapshot is otherwise reserved for a table with no snapshot.

This change resolves the explicit or latest snapshot once before creating GlobalIndexScan, passes that same ID to the scan, and returns a zero-split plan carrying the resolved ID on a miss. The fast-miss path still does not invoke the data batch scan or read data manifests. A truly empty table continues to return an empty plan with no snapshot.

For a pre-supplied empty GlobalIndexResult, the existing ScanContext remains the snapshot source: an explicit scan snapshot is preserved, otherwise the current latest snapshot is resolved when the plan is created.

Tests

  • GlobalIndexTest.TestDataEvolutionBatchScan and GlobalIndexTest.TestDataEvolutionGlobalIndexMissPreservesResolvedSnapshot: 8/8 parameterized cases passed.
  • DataEvolutionBatchScanTest.* and TableScanTest.TestNoSnapshot: 4/4 cases passed.
  • clang-format 20.1.8 dry-run/Werror, cpplint 2.0, codespell, and git diff --check passed.

The new test covers internally evaluated and pre-supplied empty index results with both latest and explicit snapshots. TableScanTest.TestNoSnapshot keeps the empty-table boundary covered.

API and Format

No public API, ABI, storage-format, or protocol change. The modified scan helpers are private implementation details and no class data member is added.

Documentation

No new feature or user-facing configuration; no documentation change is required.

Generative AI tooling

Generated-by: OpenAI Codex (GPT-5)

std::optional<int64_t> snapshot_id = core_options_.GetScanSnapshotId();
if (snapshot_id) {
return snapshot_id;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please validate the snapshot before returning the empty plan. With a pre-supplied empty GlobalIndexResult, a nonexistent scan.snapshot-id (for example, 999) is currently copied directly into a successful plan. Please resolve/load the snapshot here and add a regression test for nonexistent or expired snapshot IDs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior seems to predate the PR: the global-index scan already appears to ignore tag- and timestamp-based time travel and fall back to the latest snapshot. What changes here is that the new resolver makes this more visible by attaching that latest snapshot ID to an empty plan. Since global-index time travel does not seem to be supported at the moment, would it make sense to reject these modes explicitly instead of silently scanning and reporting the latest snapshot?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants