github-actions[bot] commented on code in PR #67343:
URL: https://github.com/apache/doris/pull/67343#discussion_r3901695392
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/LogicalResultSinkToShortCircuitPointQuery.java:
##########
@@ -98,8 +109,10 @@ public List<Rule> buildRules() {
.when(this::scanMatchShortCircuitCondition)
).when(this::filterMatchShortCircuitCondition)))
.thenApply(ctx -> {
+ if (projectReadsDictionary(ctx.root.child())) {
Review Comment:
[P1] Preserve dictionary dependencies across constant folding
The existing prepared-reuse issue is still reachable when
`enable_fold_constant_by_be=true`. For `ResultSink ->
Project(dict_get('db.d','v',1)) -> Filter(id = ?) -> Scan(point_table)`, plan
normalization folds the all-literal dictionary call before this table/physical
rule runs (`FoldConstantRuleOnBE` does not skip either dictionary function).
This guard then sees only a `Literal`, marks the query short-circuit, and
`ShortCircuitQueryContext` can reuse that serialized old value after the
dictionary privilege or version changes. The new regression uses nonconstant
`id` and never enables BE folding, so it cannot catch this. Please preserve a
statement-level dictionary dependency through rewrites (or prevent these calls
from folding) and cover literal `dict_get`/`dict_get_many` with both
revoke-after-prime and refresh-after-prime prepared executions.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/refresh/RefreshDictionaryCommand.java:
##########
@@ -45,7 +54,28 @@ public RefreshDictionaryCommand(String dbName, String
dicName) {
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
DictionaryManager dictionaryManager =
ctx.getEnv().getDictionaryManager();
String db = dbName == null ? ctx.getDatabase() : dbName;
+ // The reload is an INSERT INTO the dictionary executed as the current
user, which requires
+ // LOAD on the dictionary (and SELECT on the source table). Check LOAD
up front so a user
+ // without it can neither probe whether the dictionary exists nor flip
it to LOADING.
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
InternalCatalog.INTERNAL_CATALOG_NAME,
+ db, dictionaryName, PrivPredicate.LOAD)) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"LOAD",
+ ctx.getQualifiedUser(), ctx.getRemoteIP(), db + ": " +
dictionaryName);
+ }
Dictionary dictionary = dictionaryManager.getDictionary(db,
dictionaryName);
+ // The reload also reads the source table as the current user, and its
INSERT authorizes
+ // exactly the dictionary's source columns through the column-aware
contract (BindSink
+ // projects the source to the dictionary schema before CheckPrivileges
runs). Use the same
+ // contract and the same column spelling here: the dictionary
definition may spell a column
+ // differently from the source table, but column grants are compared
against the source
+ // column's own name. Reject a missing privilege before dataLoad()
publishes the LOADING
+ // status and blocks concurrent refreshes.
+ Set<String> sourceColumns = dictionary.getDicColumns().stream()
+ .map(definition -> definition.getOriginColumn().getName())
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ Env.getCurrentEnv().getAccessManager().checkColumnsPriv(ctx,
dictionary.getSourceCtlName(),
+ dictionary.getSourceDbName(), dictionary.getSourceTableName(),
sourceColumns,
Review Comment:
[P2] Use the resolved source table name in this preflight
The column set is canonical now, but the table argument still comes from the
spelling persisted by `CREATE DICTIONARY`. Under `lower_case_table_names=1/2`,
that spelling can differ from the resolved `Table.getName()`: native
`ColPrivilegeKey` compares the table string exactly, while the generated
INSERT's `UserAuthentication` check uses the resolved table name. A canonical
column grant can therefore be rejected here even though the INSERT would pass;
the inverse spelling can pass here and fail only after `dataLoad()` publishes
`LOADING`. Please authorize with the resolved source qualifiers (or persist
them canonically) and add a mixed-case source-table regression with column-only
grants.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DictGet.java:
##########
@@ -93,6 +96,16 @@ public Pair<FunctionSignature, Dictionary>
customSignatureDict() {
String dictName = firstNames[1];
String colName = ((Literal) getArgument(1)).getStringValue();
+ // Reading dictionary values must be authorized like reading its
source data. Check before
+ // the lookup so a caller without the privilege cannot even probe
whether the dictionary
+ // exists. ConnectContext may be absent on internal paths, which carry
no user to check.
+ ConnectContext connectContext = ConnectContext.get();
+ if (connectContext != null &&
!Env.getCurrentEnv().getAccessManager().checkTblPriv(connectContext,
Review Comment:
[P1] Preserve the persisted-view privilege boundary
This invocation-time check also runs while `BindRelation` reparses a
persisted view definition under the querying user's `ConnectContext`, before
the analyzed body is wrapped in `LogicalView`. A user granted only `SELECT` on
`db.v` therefore cannot execute `SELECT` against a view created as `SELECT
dict_get('db.d','v',1)`, even though `CheckPrivileges.visitLogicalView`
explicitly authorizes the view and stops before its protected children (the
established behavior covered by `test_select_view_auth`). `DictGetMany` has the
same check, and translation calls this helper again after view inlining. Please
preserve the direct-call and CREATE/ALTER checks while carrying a trusted
persisted-view scope through binding and translation, and add view-only
regressions for both functions plus a direct-call negative control.
--
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]
