morrySnow commented on code in PR #44755:
URL: https://github.com/apache/doris/pull/44755#discussion_r1889578249
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -770,41 +770,38 @@ public LogicalPlan
visitCreateScheduledJob(DorisParser.CreateScheduledJobContext
return new CreateJobCommand(createJobInfo);
}
+ private void checkJobNameKey(String key, String keyFormat) {
+ if (key.isEmpty() || !key.equalsIgnoreCase(keyFormat)) {
Review Comment:
why add key.isEmpty()?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -770,41 +770,38 @@ public LogicalPlan
visitCreateScheduledJob(DorisParser.CreateScheduledJobContext
return new CreateJobCommand(createJobInfo);
}
+ private void checkJobNameKey(String key, String keyFormat) {
+ if (key.isEmpty() || !key.equalsIgnoreCase(keyFormat)) {
+ throw new ParseException(keyFormat + " should be: '" + keyFormat +
"'");
Review Comment:
error message is not very clear, we should use ParseException with Origin,
or different statement use different error message to let user could know the
error reason easily
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -770,41 +770,38 @@ public LogicalPlan
visitCreateScheduledJob(DorisParser.CreateScheduledJobContext
return new CreateJobCommand(createJobInfo);
}
+ private void checkJobNameKey(String key, String keyFormat) {
+ if (key.isEmpty() || !key.equalsIgnoreCase(keyFormat)) {
+ throw new ParseException(keyFormat + " should be: '" + keyFormat +
"'");
+ }
+ }
+
@Override
public LogicalPlan visitPauseJob(DorisParser.PauseJobContext ctx) {
- Expression wildWhere = null;
- if (ctx.wildWhere() != null) {
- wildWhere = getWildWhere(ctx.wildWhere());
- }
- return new PauseJobCommand(wildWhere);
+ checkJobNameKey(stripQuotes(ctx.jobNameKey.getText()), "jobName");
+ return new PauseJobCommand(stripQuotes(ctx.jobNameValue.getText()));
}
@Override
public LogicalPlan visitDropJob(DorisParser.DropJobContext ctx) {
- Expression wildWhere = null;
- if (ctx.wildWhere() != null) {
- wildWhere = getWildWhere(ctx.wildWhere());
- }
+ checkJobNameKey(stripQuotes(ctx.jobNameKey.getText()), "jobName");
boolean ifExists = ctx.EXISTS() != null;
- return new DropJobCommand(wildWhere, ifExists);
+ return new DropJobCommand(stripQuotes(ctx.jobNameValue.getText()),
ifExists);
}
@Override
public LogicalPlan visitResumeJob(DorisParser.ResumeJobContext ctx) {
- Expression wildWhere = null;
- if (ctx.wildWhere() != null) {
- wildWhere = getWildWhere(ctx.wildWhere());
- }
- return new ResumeJobCommand(wildWhere);
+ checkJobNameKey(stripQuotes(ctx.jobNameKey.getText()), "jobName");
+ return new ResumeJobCommand(stripQuotes(ctx.jobNameValue.getText()));
}
@Override
public LogicalPlan visitCancelJobTask(DorisParser.CancelJobTaskContext
ctx) {
- Expression wildWhere = null;
- if (ctx.wildWhere() != null) {
- wildWhere = getWildWhere(ctx.wildWhere());
- }
- return new CancelJobTaskCommand(wildWhere);
+ checkJobNameKey(stripQuotes(ctx.jobNameKey.getText()), "jobName");
+ checkJobNameKey(stripQuotes(ctx.taskIdKey.getText()), "taskId");
Review Comment:
i don't think must write jobName before taskId is a good idea. since we use
where here, we should support `jobName = xxx and taskId = xx` and `taskId = xx
and jobName = xxx`
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
