Skip to content

refactor(table)!: make reserved properties immutable before v1 - #2326

Merged
zeroshade merged 7 commits into
apache:mainfrom
fallintoplace:refactor/reserved-properties-immutable-upstream-20261024
Oct 26, 2026
Merged

zeroshade merged 7 commits into
apache:mainfrom
fallintoplace:refactor/reserved-properties-immutable-upstream-20261024

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Make ReservedProperties return a fresh slice.

Why

  • Callers should not be able to change metadata validation.

Implementation

  • Return a new list and update internal checks.

Refs #2062.

Return defensive reserved property lists so consumers cannot alter table metadata validation.

Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
@fallintoplace
fallintoplace force-pushed the refactor/reserved-properties-immutable-upstream-20261024 branch from a64d4b3 to 0539be8 Compare October 25, 2026 11:06
Comment thread website/src/releases.md

- **Table purge is required on catalogs.** The `catalog.Catalog` interface now embeds `catalog.PurgeableCatalog`; every custom catalog implementation must provide `PurgeTable(ctx, identifier) error`. The narrow `catalog.PurgeableTable` interface is renamed to `catalog.PurgeableCatalog`. Built-in purge behavior is unchanged.
- **Unify statistics blob types with Puffin.** Replace `table.BlobType` with `puffin.BlobType` when constructing `table.BlobMetadata`. Replace `table.BlobTypeApacheDatasketchesThetaV1` with `puffin.BlobTypeDataSketchesTheta` and `table.BlobTypeDeletionVectorV1` with `puffin.BlobTypeDeletionVector`. Use `puffin.BlobType.IsKnown()` to identify the types defined by the Puffin specification; the redundant `table.BlobType.IsValid()` method is removed. Unknown non-empty blob type names still round-trip through table metadata.
- **Reserved table properties now use a read-only API.** Replace references to `table.ReservedProperties` with `table.ReservedProperties()`. The function returns a fresh slice, so callers cannot change metadata validation by modifying it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's keep this release note.

…eam-20261024

Restore the compatibility note from 0539be8 dropped by the release-notes sync.

@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.

The exported [9]string let any caller rewrite the reserved list and silently change SetProperties/RemoveProperties validation process-wide, so hiding it behind ReservedProperties() returning a clone is a real fix. Iterating the unexported array internally avoids the copy, and TestReservedPropertiesCannotBeMutated pins it. I pushed a merge of main (dc116c0) that restores the compatibility note from 0539be8, which addresses @badalprasadsingh's request. go vet ./... (including -tags integration) and go test ./table/ pass. LGTM.

@zeroshade

Copy link
Copy Markdown
Member

@badalprasadsingh I pushed a merge of main (dc116c0) that restores the ReservedProperties() compatibility note from 0539be8, which was the change you requested. Could you take another look when you get a chance?

@badalprasadsingh badalprasadsingh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm :) the release note is back. Though the #2318 conflict needs to be resolved: keeping both the bullets in release notes.

cc: @zeroshade

@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.

Re-approving after merging main again (81d66a0), keeping both releases.md notes; @badalprasadsingh's approval covers the restored note. go vet ./... (including -tags integration) and go test ./table/ pass. LGTM.

@zeroshade
zeroshade merged commit e6b3f8a into apache:main Oct 26, 2026
16 checks passed
zeroshade added a commit to zeroshade/iceberg-go that referenced this pull request Oct 26, 2026
Keep the pre-v1 rename notes alongside the notes from apache#2326 and apache#2331.
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.

3 participants