Skip to content

Commit f4b556b

Browse files
VijayShekhawat7Vijay Shekhawat
VijayShekhawat7
authored and
Vijay Shekhawat
committed
[BugFix] Fix GRANT on ALL VIEWS/MVs failing for external catalogs
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 Add test cases for GRANT/REVOKE on ALL VIEWS, ALL MATERIALIZED VIEWS, and ALL TABLES in external catalog databases for regression coverage. Fixes #71211 Signed-off-by: Vijay Shekhawat <vijayshekhawat1995@gmail.com> Made-with: Cursor
1 parent 4437eef commit f4b556b

4 files changed

Lines changed: 182 additions & 51 deletions

File tree

‎fe/fe-core/src/main/java/com/starrocks/authorization/MaterializedViewPEntryObject.java‎

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,51 +14,81 @@
1414

1515
package com.starrocks.authorization;
1616

17-
import com.starrocks.catalog.Database;
17+
import com.starrocks.catalog.Catalog;
18+
import com.starrocks.catalog.InternalCatalog;
1819
import com.starrocks.catalog.Table;
20+
import com.starrocks.server.CatalogMgr;
1921
import com.starrocks.server.GlobalStateMgr;
2022

2123
import java.util.List;
2224
import java.util.Objects;
2325

2426
public class MaterializedViewPEntryObject extends TablePEntryObject {
2527

28+
protected MaterializedViewPEntryObject(long catalogId, String dbUUID, String tblUUID) {
29+
super(catalogId, dbUUID, tblUUID);
30+
}
31+
2632
protected MaterializedViewPEntryObject(String dbUUID, String tblUUID) {
2733
super(dbUUID, tblUUID);
2834
}
2935

3036
public static MaterializedViewPEntryObject generate(List<String> tokens)
3137
throws PrivilegeException {
32-
if (tokens.size() != 2) {
33-
throw new PrivilegeException("invalid object tokens, should have two: " + tokens);
38+
String catalogName = null;
39+
long catalogId;
40+
if (tokens.size() == 3) {
41+
if (tokens.get(0).equals("*")) {
42+
return new MaterializedViewPEntryObject(PrivilegeBuiltinConstants.ALL_CATALOGS_ID,
43+
PrivilegeBuiltinConstants.ALL_DATABASES_UUID, PrivilegeBuiltinConstants.ALL_TABLES_UUID);
44+
}
45+
catalogName = tokens.get(0);
46+
tokens = tokens.subList(1, tokens.size());
47+
} else if (tokens.size() != 2) {
48+
throw new PrivilegeException(
49+
"invalid object tokens, should have two or three elements, current: " + tokens);
3450
}
35-
String dbUUID;
36-
String tblUUID;
3751

38-
if (Objects.equals(tokens.get(0), "*")) {
39-
dbUUID = PrivilegeBuiltinConstants.ALL_DATABASES_UUID;
40-
tblUUID = PrivilegeBuiltinConstants.ALL_TABLES_UUID;
52+
if (catalogName == null || CatalogMgr.isInternalCatalog(catalogName)) {
53+
catalogName = InternalCatalog.DEFAULT_INTERNAL_CATALOG_NAME;
54+
catalogId = InternalCatalog.DEFAULT_INTERNAL_CATALOG_ID;
4155
} else {
42-
Database database = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb(tokens.get(0));
43-
if (database == null) {
44-
throw new PrivObjNotFoundException("cannot find db: " + tokens.get(0));
56+
Catalog catalog = GlobalStateMgr.getCurrentState().getCatalogMgr().getCatalogByName(catalogName);
57+
if (catalog == null) {
58+
throw new PrivObjNotFoundException("cannot find catalog: " + catalogName);
4559
}
46-
dbUUID = database.getUUID();
60+
catalogId = catalog.getId();
61+
}
62+
63+
if (Objects.equals(tokens.get(0), "*")) {
64+
return new MaterializedViewPEntryObject(
65+
catalogId,
66+
PrivilegeBuiltinConstants.ALL_DATABASES_UUID,
67+
PrivilegeBuiltinConstants.ALL_TABLES_UUID);
68+
}
69+
70+
String dbUUID = DbPEntryObject.getDatabaseUUID(catalogName, tokens.get(0));
71+
String tblUUID = getMaterializedViewUUID(catalogName, tokens.get(0), tokens.get(1));
72+
return new MaterializedViewPEntryObject(catalogId, dbUUID, tblUUID);
73+
}
74+
75+
private static String getMaterializedViewUUID(String catalogName, String dbToken, String mvToken)
76+
throws PrivObjNotFoundException {
77+
if (mvToken.equals("*")) {
78+
return PrivilegeBuiltinConstants.ALL_TABLES_UUID;
79+
}
4780

48-
if (Objects.equals(tokens.get(1), "*")) {
49-
tblUUID = PrivilegeBuiltinConstants.ALL_TABLES_UUID;
50-
} else {
51-
Table table = GlobalStateMgr.getCurrentState().getLocalMetastore()
52-
.getTable(database.getFullName(), tokens.get(1));
53-
if (table == null || !table.isMaterializedView()) {
54-
throw new PrivObjNotFoundException(
55-
"cannot find materialized view " + tokens.get(1) + " in db " + tokens.get(0));
56-
}
57-
tblUUID = table.getUUID();
81+
if (CatalogMgr.isInternalCatalog(catalogName)) {
82+
Table table = GlobalStateMgr.getCurrentState().getLocalMetastore()
83+
.getTable(dbToken, mvToken);
84+
if (table == null || !table.isMaterializedView()) {
85+
throw new PrivObjNotFoundException(
86+
"cannot find materialized view " + mvToken + " in db " + dbToken);
5887
}
88+
return table.getUUID();
5989
}
6090

61-
return new MaterializedViewPEntryObject(dbUUID, tblUUID);
91+
return mvToken;
6292
}
6393

6494
@Override
@@ -68,6 +98,6 @@ public String toString() {
6898

6999
@Override
70100
public MaterializedViewPEntryObject clone() {
71-
return new MaterializedViewPEntryObject(this.databaseUUID, this.tableUUID);
101+
return new MaterializedViewPEntryObject(getCatalogId(), this.databaseUUID, this.tableUUID);
72102
}
73103
}

‎fe/fe-core/src/main/java/com/starrocks/authorization/ViewPEntryObject.java‎

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414

1515
package com.starrocks.authorization;
1616

17-
import com.starrocks.catalog.Database;
17+
import com.starrocks.catalog.Catalog;
18+
import com.starrocks.catalog.InternalCatalog;
1819
import com.starrocks.catalog.Table;
20+
import com.starrocks.server.CatalogMgr;
1921
import com.starrocks.server.GlobalStateMgr;
2022

2123
import java.util.List;
@@ -25,40 +27,68 @@
2527
* View is a subclass of table, only the table type is different
2628
*/
2729
public class ViewPEntryObject extends TablePEntryObject {
30+
protected ViewPEntryObject(long catalogId, String dbUUID, String tblUUID) {
31+
super(catalogId, dbUUID, tblUUID);
32+
}
33+
2834
protected ViewPEntryObject(String dbUUID, String tblUUID) {
2935
super(dbUUID, tblUUID);
3036
}
3137

3238
public static ViewPEntryObject generate(List<String> tokens) throws PrivilegeException {
33-
if (tokens.size() != 2) {
34-
throw new PrivilegeException("invalid object tokens, should have two: " + tokens);
39+
String catalogName = null;
40+
long catalogId;
41+
if (tokens.size() == 3) {
42+
if (tokens.get(0).equals("*")) {
43+
return new ViewPEntryObject(PrivilegeBuiltinConstants.ALL_CATALOGS_ID,
44+
PrivilegeBuiltinConstants.ALL_DATABASES_UUID, PrivilegeBuiltinConstants.ALL_TABLES_UUID);
45+
}
46+
catalogName = tokens.get(0);
47+
tokens = tokens.subList(1, tokens.size());
48+
} else if (tokens.size() != 2) {
49+
throw new PrivilegeException(
50+
"invalid object tokens, should have two or three elements, current: " + tokens);
3551
}
36-
String dbUUID;
37-
String tblUUID;
3852

39-
if (Objects.equals(tokens.get(0), "*")) {
40-
dbUUID = PrivilegeBuiltinConstants.ALL_DATABASES_UUID;
41-
tblUUID = PrivilegeBuiltinConstants.ALL_TABLES_UUID;
53+
if (catalogName == null || CatalogMgr.isInternalCatalog(catalogName)) {
54+
catalogName = InternalCatalog.DEFAULT_INTERNAL_CATALOG_NAME;
55+
catalogId = InternalCatalog.DEFAULT_INTERNAL_CATALOG_ID;
4256
} else {
43-
Database database = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb(tokens.get(0));
44-
if (database == null) {
45-
throw new PrivObjNotFoundException("cannot find db: " + tokens.get(0));
57+
Catalog catalog = GlobalStateMgr.getCurrentState().getCatalogMgr().getCatalogByName(catalogName);
58+
if (catalog == null) {
59+
throw new PrivObjNotFoundException("cannot find catalog: " + catalogName);
4660
}
47-
dbUUID = database.getUUID();
61+
catalogId = catalog.getId();
62+
}
63+
64+
if (Objects.equals(tokens.get(0), "*")) {
65+
return new ViewPEntryObject(
66+
catalogId,
67+
PrivilegeBuiltinConstants.ALL_DATABASES_UUID,
68+
PrivilegeBuiltinConstants.ALL_TABLES_UUID);
69+
}
70+
71+
String dbUUID = DbPEntryObject.getDatabaseUUID(catalogName, tokens.get(0));
72+
String tblUUID = getViewUUID(catalogName, tokens.get(0), tokens.get(1));
73+
return new ViewPEntryObject(catalogId, dbUUID, tblUUID);
74+
}
75+
76+
private static String getViewUUID(String catalogName, String dbToken, String viewToken)
77+
throws PrivObjNotFoundException {
78+
if (viewToken.equals("*")) {
79+
return PrivilegeBuiltinConstants.ALL_TABLES_UUID;
80+
}
4881

49-
if (Objects.equals(tokens.get(1), "*")) {
50-
tblUUID = PrivilegeBuiltinConstants.ALL_TABLES_UUID;
51-
} else {
52-
Table table = GlobalStateMgr.getCurrentState().getLocalMetastore()
53-
.getTable(database.getFullName(), tokens.get(1));
54-
if (table == null || !table.isOlapView()) {
55-
throw new PrivObjNotFoundException("cannot find view " + tokens.get(1) + " in db " + tokens.get(0));
56-
}
57-
tblUUID = table.getUUID();
82+
if (CatalogMgr.isInternalCatalog(catalogName)) {
83+
Table table = GlobalStateMgr.getCurrentState().getLocalMetastore()
84+
.getTable(dbToken, viewToken);
85+
if (table == null || !table.isOlapView()) {
86+
throw new PrivObjNotFoundException("cannot find view " + viewToken + " in db " + dbToken);
5887
}
88+
return table.getUUID();
5989
}
6090

61-
return new ViewPEntryObject(dbUUID, tblUUID);
91+
return viewToken;
6292
}
6393

6494
@Override
@@ -68,6 +98,6 @@ public String toString() {
6898

6999
@Override
70100
public ViewPEntryObject clone() {
71-
return new ViewPEntryObject(this.databaseUUID, this.tableUUID);
101+
return new ViewPEntryObject(getCatalogId(), this.databaseUUID, this.tableUUID);
72102
}
73103
}

‎fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AuthorizationAnalyzer.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,13 @@ public List<List<String>> analyzeTokens(BaseGrantRevokePrivilegeStmt stmt, Objec
300300
}
301301
objectTokenList.add(Lists.newArrayList(session.getCurrentCatalog(), tokens.get(0), tokens.get(1)));
302302
} else if (ObjectType.VIEW.equals(objectType)
303-
|| ObjectType.MATERIALIZED_VIEW.equals(objectType)
304-
|| ObjectType.PIPE.equals(objectType)) {
303+
|| ObjectType.MATERIALIZED_VIEW.equals(objectType)) {
304+
if (tokens.size() != 2) {
305+
throw new SemanticException(
306+
"Invalid grant statement with error privilege object " + tokens);
307+
}
308+
objectTokenList.add(Lists.newArrayList(session.getCurrentCatalog(), tokens.get(0), tokens.get(1)));
309+
} else if (ObjectType.PIPE.equals(objectType)) {
305310
if (tokens.size() != 2) {
306311
throw new SemanticException(
307312
"Invalid grant statement with error privilege object " + tokens);
@@ -352,8 +357,11 @@ public List<List<String>> analyzeTokens(BaseGrantRevokePrivilegeStmt stmt, Objec
352357

353358
for (List<String> tokens : stmt.getPrivilegeObjectNameTokensList()) {
354359
TableName tableName;
355-
if (tokens.size() == 2) {
360+
if (tokens.size() == 3) {
361+
tableName = new TableName(tokens.get(0), tokens.get(1), tokens.get(2));
362+
} else if (tokens.size() == 2) {
356363
tableName = new TableName(tokens.get(0), tokens.get(1));
364+
tableName.normalization(session);
357365
} else if (tokens.size() == 1) {
358366
tableName = new TableName("", tokens.get(0));
359367
tableName.normalization(session);
@@ -362,7 +370,8 @@ public List<List<String>> analyzeTokens(BaseGrantRevokePrivilegeStmt stmt, Objec
362370
"Invalid grant statement with error privilege object " + tokens);
363371
}
364372

365-
objectTokenList.add(Lists.newArrayList(tableName.getDb(), tableName.getTbl()));
373+
objectTokenList.add(Lists.newArrayList(
374+
tableName.getCatalog(), tableName.getDb(), tableName.getTbl()));
366375
}
367376
} else if (ObjectType.PIPE.equals(objectType)) {
368377
Preconditions.checkArgument(stmt.getPrivilegeObjectNameTokensList() != null);

‎fe/fe-core/src/test/java/com/starrocks/sql/analyzer/PrivilegeStmtAnalyzerV2Test.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.starrocks.sql.ast.SetRoleType;
3636
import com.starrocks.sql.ast.StatementBase;
3737
import com.starrocks.sql.ast.UserRef;
38+
import com.starrocks.sql.plan.ConnectorPlanTestBase;
3839
import com.starrocks.utframe.StarRocksAssert;
3940
import com.starrocks.utframe.UtFrameUtils;
4041
import org.junit.jupiter.api.AfterAll;
@@ -72,6 +73,7 @@ public static void setUp() throws Exception {
7273
CreateUserStmt createUserStmt = (CreateUserStmt) UtFrameUtils.parseStmtWithNewParser(
7374
"create user test_user", ctx);
7475
ctx.getGlobalStateMgr().getAuthenticationMgr().createUser(createUserStmt);
76+
ConnectorPlanTestBase.mockHiveCatalog(ctx);
7577
}
7678

7779
@AfterAll
@@ -829,4 +831,64 @@ public void testErrorParam() {
829831
"Invalid grant statement with error privilege object");
830832

831833
}
834+
835+
@Test
836+
public void testGrantAllViewsInExternalCatalogDatabase() throws Exception {
837+
String previousCatalog = ctx.getCurrentCatalog();
838+
try {
839+
ctx.setCurrentCatalog("hive0");
840+
841+
String sql = "grant select on ALL views in database tpch to test_user";
842+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
843+
844+
sql = "revoke select on ALL views in database tpch from test_user";
845+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
846+
847+
sql = "grant select on ALL views in all databases to test_user";
848+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
849+
850+
sql = "revoke select on ALL views in all databases from test_user";
851+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
852+
} finally {
853+
ctx.setCurrentCatalog(previousCatalog);
854+
}
855+
}
856+
857+
@Test
858+
public void testGrantAllMaterializedViewsInExternalCatalogDatabase() throws Exception {
859+
String previousCatalog = ctx.getCurrentCatalog();
860+
try {
861+
ctx.setCurrentCatalog("hive0");
862+
863+
String sql = "grant select on ALL materialized views in database tpch to test_user";
864+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
865+
866+
sql = "revoke select on ALL materialized views in database tpch from test_user";
867+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
868+
869+
sql = "grant select on ALL materialized views in all databases to test_user";
870+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
871+
872+
sql = "revoke select on ALL materialized views in all databases from test_user";
873+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
874+
} finally {
875+
ctx.setCurrentCatalog(previousCatalog);
876+
}
877+
}
878+
879+
@Test
880+
public void testGrantAllTablesInExternalCatalogDatabase() throws Exception {
881+
String previousCatalog = ctx.getCurrentCatalog();
882+
try {
883+
ctx.setCurrentCatalog("hive0");
884+
885+
String sql = "grant select on ALL tables in database tpch to test_user";
886+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
887+
888+
sql = "revoke select on ALL tables in database tpch from test_user";
889+
Assertions.assertNotNull(UtFrameUtils.parseStmtWithNewParser(sql, ctx));
890+
} finally {
891+
ctx.setCurrentCatalog(previousCatalog);
892+
}
893+
}
832894
}

0 commit comments

Comments
 (0)