Skip to content

Skip location overlap validation when namespace location is unchanged - #5172

Open
ayushtkn wants to merge 1 commit into
apache:mainfrom
ayushtkn:nsOverlapSkip
Open

Skip location overlap validation when namespace location is unchanged#5172
ayushtkn wants to merge 1 commit into
apache:mainfrom
ayushtkn:nsOverlapSkip

Conversation

@ayushtkn

@ayushtkn ayushtkn commented Jul 28, 2026

Copy link
Copy Markdown
Member

updateNamespaceProperties runs the sibling location-overlap validation on every namespace property update, even when the update leaves the namespace location untouched.

That check scales with the number of sibling tables and namespaces, so a trivial update such as setting owner can do a significant amount of work purely to verify a base location that did not change. This change gates the check on the location actually changing, which is how the table and view commit paths already gate the same validation, and leaves behaviour identical whenever the location is modified.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

Copilot AI review requested due to automatic review settings July 28, 2026 03:44
@github-project-automation github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Jul 28, 2026

Copilot AI 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.

Pull request overview

This PR reduces unnecessary work when updating namespace properties by skipping sibling location-overlap validation unless the namespace location actually changes, aligning namespace updates with the existing table/view validation behavior.

Changes:

  • Gate namespace sibling location-overlap validation in LocalIcebergCatalog#setProperties on whether the namespace base location changed.
  • Add a regression test that asserts overlap validation work (via metastore listEntities calls) is performed only when location is updated.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java Skips location-overlap validation on namespace property updates when the base location is unchanged.
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java Adds a test to verify overlap validation is triggered only on namespace location changes.

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

nits: missing / in the namespace location format string, and the listEntities spy may be brittle if OPTIMIZED_SIBLING_CHECK is on - consider pinning it false... WDYT ??

@github-project-automation github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jul 28, 2026
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.

5 participants