Default PURGE_VIEW_METADATA_ON_DROP to false so views can be dropped out of the box - #5335
Closed
zhang-arvin wants to merge 2 commits into
Closed
Default PURGE_VIEW_METADATA_ON_DROP to false so views can be dropped out of the box#5335zhang-arvin wants to merge 2 commits into
zhang-arvin wants to merge 2 commits into
Conversation
…ling-check query The optimized sibling check reads location_without_scheme via QueryGenerator.generateOverlapQuery, which filters realm_id and catalog_id. On Postgres and CockroachDB the idx_locations index led with parent_id instead of catalog_id, so the predicate could only probe the realm_id prefix and every CREATE TABLE / CREATE NAMESPACE with OPTIMIZED_SIBLING_CHECK enabled fell back to a scan instead of the intended indexed lookup. H2 already used the correct columns. Schema v5 has already shipped in 1.7.0, so rather than editing a released script this adds schema v6 for all backends with the corrected index and bumps the latest schema version. Fresh installs bootstrap at v6 and get the right index automatically. Existing deployments need a one-time manual index recreation, documented in the changelog upgrade notes, because Polaris has no automated schema migrations.
…out of the box Fixes: apache#5293 The default values of DROP_WITH_PURGE_ENABLED (false) and PURGE_VIEW_METADATA_ON_DROP (true) were incompatible: dropping a view derived purge=true from the view flag, but the guard rejected purge=true because the table flag was off. As a result, views could not be dropped under the default configuration. Change the default of PURGE_VIEW_METADATA_ON_DROP to false so that views can be dropped out of the box. Deployments that rely on view metadata being purged should explicitly set PURGE_VIEW_METADATA_ON_DROP to true alongside DROP_WITH_PURGE_ENABLED. - Update FeatureConfiguration.java default value - Add integration test for default config behavior - Update CHANGELOG.md and documentation
Member
|
this dupes an already existing PR linked in the issue: #5322 |
Contributor
|
@zhang-arvin Before opening a PR, please check if there’s already an open PR for the same issue. If there is, please avoid creating a duplicate and consider contributing to the existing PR instead. I can also suggest some good first issues if you’d like to pick up another one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #5293
Problem
The default values of
DROP_WITH_PURGE_ENABLED(false) andPURGE_VIEW_METADATA_ON_DROP(true) were incompatible: dropping a view derivedpurge=truefrom the view flag, but the guard rejectedpurge=truebecause the table flag was off. As a result, views could not be dropped under the default configuration, returning a 403ForbiddenException.Root Cause
In
LocalIcebergCatalog.dropView, thepurgeparameter is derived fromPURGE_VIEW_METADATA_ON_DROP(defaulttrue). This is passed todropTableLike, whose guard checksDROP_WITH_PURGE_ENABLED(defaultfalse). The shipped defaults are exactly the combination that fails.Fix
Change the default value of
PURGE_VIEW_METADATA_ON_DROPfromtruetofalse, so that views can be dropped out of the box without requiringDROP_WITH_PURGE_ENABLEDto be enabled.Changes
FeatureConfiguration.java: ChangePURGE_VIEW_METADATA_ON_DROPdefault fromtruetofalse, update description to note dependency onDROP_WITH_PURGE_ENABLEDPolarisRestCatalogIntegrationBase.java: AddtestDropViewWithDefaultPurgeViewMetadataOnDroptest verifying views can be dropped whenPURGE_VIEW_METADATA_ON_DROPis unset (default applies)CHANGELOG.md: Document the breaking/default behavior changeflags-polaris_features.md: Update documented default valueUpgrade Guidance
Deployments that enable
DROP_WITH_PURGE_ENABLEDand rely on view metadata being purged should now explicitly setPURGE_VIEW_METADATA_ON_DROPtotrue.