Skip to content

refactor(table)!: make SnapshotAsOf inclusive before v1 - #2322

Merged
zeroshade merged 5 commits into
apache:mainfrom
fallintoplace:refactor/snapshot-asof-inclusive-upstream-20261024
Oct 26, 2026
Merged

zeroshade merged 5 commits into
apache:mainfrom
fallintoplace:refactor/snapshot-asof-inclusive-upstream-20261024

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Remove the inclusive argument from Table.SnapshotAsOf.

Why

  • An as-of lookup includes snapshots at the requested timestamp.

Implementation

  • Make the public lookup inclusive and keep strict-before support in the lower-level helper.

Refs #2062.

Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
@fallintoplace
fallintoplace force-pushed the refactor/snapshot-asof-inclusive-upstream-20261024 branch from 9d42bce to a70c1fb Compare October 25, 2026 11:15
fallintoplace and others added 4 commits October 25, 2026 15:42
…261024

Restore the SnapshotAsOf compatibility note from a70c1fb dropped by the release-notes sync.
…261024

Take main's releases.md; the compatibility note for this change lands in a follow-up release-notes PR so the pre-v1 renames can merge in parallel.

@zeroshade zeroshade left a comment

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.

An as-of lookup should include the requested timestamp, and Java's SnapshotUtil.nullableSnapshotIdAsOfTime does the same. The only non-test caller already passed true, and WithSnapshotAsOf and inspect were already inclusive, so behavior doesn't change. Strict-before stays available through SnapshotLogEntryAsOf, and TestSnapshotLogEntryAsOf still covers it, so dropping the SnapshotAsOf(..., false) tests loses no coverage. I pushed a merge of main (94a6fbe); go vet ./... (including -tags integration) and the table and CLI tests pass. The compatibility note for this change (dropped by the "sync release notes with main" commit) will land in a single follow-up release-notes PR covering this batch of pre-v1 renames, so these can merge in parallel without releases.md conflicts. LGTM.

@zeroshade
zeroshade merged commit 25ca505 into apache:main Oct 26, 2026
16 checks passed
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