mrhhsg commented on code in PR #67343:
URL: https://github.com/apache/doris/pull/67343#discussion_r3893372571


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/refresh/RefreshDictionaryCommand.java:
##########
@@ -18,8 +18,13 @@
 package org.apache.doris.nereids.trees.plans.commands.refresh;
 
 import org.apache.doris.analysis.StmtType;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.datasource.InternalCatalog;

Review Comment:
   [P1] Dictionaries are authorized with the table privilege key of the 
internal catalog by design: `CREATE/DROP DICTIONARY` already use 
`checkTblPriv(internal, db, dictName, ...)` (#66218), and `GRANT` can only 
target `db.<name>` (there is no dictionary privilege object). This PR keeps 
that model and only adds the checks that were missing, so 
`SHOW/EXPLAIN/REFRESH` now behave consistently with `CREATE/DROP`. The 
name-collision problem is real, but it predates this change and affects 
`CREATE/DROP` the same way; fixing it means either rejecting table/dictionary 
name collisions in both `CREATE DICTIONARY` and `CREATE TABLE`, or introducing 
a dedicated privilege object for internal and Ranger controllers. That is out 
of scope here and tracked in #67345.
   
   [P2] Fixed in 9a1fbfe: after the `LOAD` check the command now also verifies 
`SELECT` on the source table (`dictionary.getSourceCtlName/DbName/TableName`, 
the same check `CREATE DICTIONARY` does) before calling `dataLoad()`, so a 
request lacking either privilege is rejected before the shared status is 
switched to `LOADING`. Covered by the new `LOAD`-without-source-`SELECT` case 
in `test_ddl_dictionary_auth`.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to