github-actions[bot] commented on code in PR #63505:
URL: https://github.com/apache/doris/pull/63505#discussion_r3292330715
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadschedpolicy/WorkloadSchedPolicyMgr.java:
##########
@@ -39,6 +39,7 @@
import org.apache.doris.thrift.TWorkloadMetricType;
import org.apache.doris.thrift.TopicInfo;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
Review Comment:
This validation still lets malformed values through because
`String.split("\\.")` discards trailing empty segments. For example, in
non-cloud mode `workload_group='wg.'` becomes `['wg']` and is accepted even
though the new contract says only `<workload_group>` with no qualifier is
allowed; in cloud mode `.wg` has length 2 and reaches the compute-group lookup
instead of being rejected as an invalid `<compute_group>.<workload_group>`
form. Please preserve empty segments, for example with `split("\\.", -1)`, and
reject empty `cg`/`wg` components before lookup; add tests for these cases.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropWorkloadGroupCommand.java:
##########
@@ -60,10 +60,11 @@ public void doRun(ConnectContext ctx, StmtExecutor
executor) throws Exception {
}
if (Config.isCloudMode()) {
- String originCgStr = computeGroup;
if (StringUtils.isEmpty(computeGroup)) {
- computeGroup = Tag.VALUE_DEFAULT_COMPUTE_GROUP_NAME;
+ throw new UserException("Must specify compute group via 'FROM
<compute_group>' "
Review Comment:
The parser still accepts `DROP WORKLOAD GROUP ... FOR <compute_group>` and
does not accept `FROM` here (`DorisParser.g4` line 354). If a cloud user omits
the clause and follows this error message, the suggested statement will fail to
parse. Please either change the grammar to support `FROM` or make this message
say `FOR <compute_group>` consistently.
```suggestion
throw new UserException("Must specify compute group via 'FOR
<compute_group>' "
```
--
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]
