Copilot commented on code in PR #56965:
URL: https://github.com/apache/doris/pull/56965#discussion_r2434333398
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterJobCommand.java:
##########
@@ -124,11 +128,18 @@ private void validate() throws Exception {
/**
* Check if there are any unmodifiable properties in TVF
*/
- private void checkUnmodifiableProperties(String originExecuteSql) {
- UnboundTVFRelation originTvf = getTvf(originExecuteSql);
- UnboundTVFRelation inputTvf = getTvf(sql);
+ private void checkUnmodifiableProperties(String originExecuteSql) throws
AnalysisException {
+ Pair<List<String>, UnboundTVFRelation> origin =
getTargetTableAndTvf(originExecuteSql);
+ Pair<List<String>, UnboundTVFRelation> input =
getTargetTableAndTvf(sql);
+ UnboundTVFRelation originTvf = origin.second;
+ UnboundTVFRelation inputTvf = input.second;
+
+ Preconditions.checkArgument(Objects.equals(origin.first, input.first),
+ "The target table cannot be modified in ALTER JOB");
+
Preconditions.checkArgument(originTvf.getFunctionName().equalsIgnoreCase(inputTvf.getFunctionName()),
- "The tvf type %s cannot be modified in ALTER JOB", inputTvf);
+ "The tvf type %s cannot be modified in ALTER JOB",
inputTvf.getFunctionName());
Review Comment:
The error message format string expects one argument but getFunctionName()
might return different values for originTvf and inputTvf. Consider including
both function names in the error message for clarity.
```suggestion
"The tvf type cannot be modified in ALTER JOB: original=%s,
new=%s",
originTvf.getFunctionName(), inputTvf.getFunctionName());
```
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -409,6 +414,50 @@ public TRow getTvfInfo() {
return trow;
}
+ private static boolean checkPrivilege(ConnectContext ctx, LogicalPlan
logicalPlan) throws AnalysisException {
+ if (!(logicalPlan instanceof InsertIntoTableCommand)) {
+ throw new AnalysisException("Only support insert command");
+ }
+ LogicalPlan logicalQuery = ((InsertIntoTableCommand)
logicalPlan).getLogicalQuery();
+ List<String> targetTable =
InsertUtils.getTargetTableQualified(logicalQuery, ctx);
+ Preconditions.checkArgument(targetTable.size() == 3, "target table
name is invalid");
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
+ InternalCatalog.INTERNAL_CATALOG_NAME,
+ targetTable.get(1),
+ targetTable.get(2),
+ PrivPredicate.LOAD)) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"LOAD",
+ ConnectContext.get().getQualifiedUser(),
+ ConnectContext.get().getRemoteIP(),
Review Comment:
Using ConnectContext.get() instead of the passed ctx parameter creates
inconsistency. Use the ctx parameter for consistency with the method signature.
```suggestion
ctx.getQualifiedUser(),
ctx.getRemoteIP(),
```
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -409,6 +414,50 @@ public TRow getTvfInfo() {
return trow;
}
+ private static boolean checkPrivilege(ConnectContext ctx, LogicalPlan
logicalPlan) throws AnalysisException {
+ if (!(logicalPlan instanceof InsertIntoTableCommand)) {
+ throw new AnalysisException("Only support insert command");
+ }
+ LogicalPlan logicalQuery = ((InsertIntoTableCommand)
logicalPlan).getLogicalQuery();
+ List<String> targetTable =
InsertUtils.getTargetTableQualified(logicalQuery, ctx);
+ Preconditions.checkArgument(targetTable.size() == 3, "target table
name is invalid");
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
+ InternalCatalog.INTERNAL_CATALOG_NAME,
+ targetTable.get(1),
+ targetTable.get(2),
+ PrivPredicate.LOAD)) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"LOAD",
+ ConnectContext.get().getQualifiedUser(),
+ ConnectContext.get().getRemoteIP(),
+ targetTable.get(1),
+ targetTable.get(2));
+ }
+ return true;
+ }
+
+ public boolean checkPrivilege(ConnectContext ctx) throws AnalysisException
{
+ LogicalPlan logicalPlan = new
NereidsParser().parseSingle(getExecuteSql());
+ return checkPrivilege(ctx, logicalPlan);
+ }
+
+ public static boolean checkPrivilege(ConnectContext ctx, String sql)
throws AnalysisException {
+ LogicalPlan logicalPlan = new NereidsParser().parseSingle(sql);
+ return checkPrivilege(ctx, logicalPlan);
+ }
+
+ public boolean hasPrivilege(UserIdentity userIdentity) {
+ ConnectContext ctx = InsertTask.makeConnectContext(userIdentity,
getCurrentDbName());
+ LogicalPlan logicalPlan = new
NereidsParser().parseSingle(getExecuteSql());
+ LogicalPlan logicalQuery = ((InsertIntoTableCommand)
logicalPlan).getLogicalQuery();
+ List<String> targetTable =
InsertUtils.getTargetTableQualified(logicalQuery, ctx);
+ Preconditions.checkArgument(targetTable.size() == 3, "target table
name is invalid");
+ return
Env.getCurrentEnv().getAccessManager().checkTblPriv(userIdentity,
+ InternalCatalog.INTERNAL_CATALOG_NAME,
+ targetTable.get(1),
+ targetTable.get(2),
+ PrivPredicate.LOAD);
Review Comment:
The created ConnectContext is not being closed or managed properly. Consider
using try-with-resources or ensure proper cleanup to prevent resource leaks.
```suggestion
try {
LogicalPlan logicalPlan = new
NereidsParser().parseSingle(getExecuteSql());
LogicalPlan logicalQuery = ((InsertIntoTableCommand)
logicalPlan).getLogicalQuery();
List<String> targetTable =
InsertUtils.getTargetTableQualified(logicalQuery, ctx);
Preconditions.checkArgument(targetTable.size() == 3, "target
table name is invalid");
return
Env.getCurrentEnv().getAccessManager().checkTblPriv(userIdentity,
InternalCatalog.INTERNAL_CATALOG_NAME,
targetTable.get(1),
targetTable.get(2),
PrivPredicate.LOAD);
} finally {
if (ctx != null) {
ctx.close();
}
}
```
--
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]
