[BugFix] Fix GRANT on ALL VIEWS/MVs failing for external catalogs#71295
Open
VijayShekhawat7 wants to merge 1 commit intoStarRocks:mainfrom
Open
[BugFix] Fix GRANT on ALL VIEWS/MVs failing for external catalogs#71295VijayShekhawat7 wants to merge 1 commit intoStarRocks:mainfrom
VijayShekhawat7 wants to merge 1 commit intoStarRocks:mainfrom
Conversation
|
|
VijayShekhawat7
force-pushed
the
fix/grant-views-external-catalog
branch
from
2a9903b to
f8388d3
Compare
VijayShekhawat7
force-pushed
the
fix/grant-views-external-catalog
branch
from
a87e993 to
f4b556b
Compare
ViewPEntryObject and MaterializedViewPEntryObject resolved databases exclusively through the local (internal) metastore, causing "cannot find db" errors when granting on views or materialized views in external catalogs such as Iceberg. The analyzer also did not prepend the current catalog name for VIEW and MATERIALIZED_VIEW object types, unlike TABLE which already did. Mirror the catalog-aware resolution pattern from TablePEntryObject: - Accept 3-element token lists [catalog, db, object] - Resolve catalog ID via CatalogMgr - Use DbPEntryObject.getDatabaseUUID() which returns the DB name directly for external catalogs without local metastore lookup - Return view/MV name directly as UUID for external catalogs - Handle ResourceMappingCatalog edge case (same as TablePEntryObject) - Add Preconditions guard and StarRocksConnectorException catch for consistency with TablePEntryObject.getTableUUID() Update initPrivilegeCollectionAllObjects to pass 3 tokens (["*","*","*"]) for VIEW and MATERIALIZED_VIEW so built-in roles (root, db_admin) receive ALL_CATALOGS_ID and correctly match external catalog objects. Add test cases for GRANT/REVOKE on ALL VIEWS, ALL MATERIALIZED VIEWS, and ALL TABLES in external catalog databases, internal catalog views, specific view grant on external catalog, and invalid catalog handling. Fixes StarRocks#71211 Signed-off-by: Vijay Shekhawat <vijayshekhawat1995@gmail.com> Made-with: Cursor
VijayShekhawat7
force-pushed
the
fix/grant-views-external-catalog
branch
from
f4b556b to
9c6a62e
Compare
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
2 participants
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.
Why I'm doing:
GRANT ALL ON ALL VIEWS IN DATABASEfails withcannot find db: <db_name>whenthe current session is connected to an external catalog (e.g., Iceberg via REST).
The equivalent
GRANT ALL ON ALL TABLES IN DATABASEworks correctly.This happens because
ViewPEntryObjectandMaterializedViewPEntryObjectresolvedatabases exclusively through the local (internal) metastore, while
TablePEntryObjectalready has catalog-aware resolution that supports externalcatalogs.
What I'm doing:
Mirror the catalog-aware resolution pattern from
TablePEntryObjectintoViewPEntryObjectandMaterializedViewPEntryObject:AuthorizationAnalyzer.analyzeTokens: Prependsession.getCurrentCatalog()for
VIEWandMATERIALIZED_VIEWobject types (matching existingTABLEbehavior) in both the ALL and non-ALL grant paths.
ViewPEntryObject.generate: Accept 3-element token lists[catalog, db, view],resolve catalog ID via
CatalogMgr, useDbPEntryObject.getDatabaseUUID()forexternal catalogs (returns DB name directly without local metastore lookup), and
return view names directly as UUIDs for external catalogs.
MaterializedViewPEntryObject.generate: Same fix asViewPEntryObject.Fixes #71211
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: