morrySnow commented on code in PR #67010:
URL: https://github.com/apache/doris/pull/67010#discussion_r3849230559
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -405,10 +409,37 @@ private void installTaskSnapshots(StatementContext
statementContext) {
}
private void setComputeGroup(ConnectContext ctx) {
- String taskComputeGroup = taskContext.getComputeGroup();
- if (Config.isCloudMode() && !Strings.isNullOrEmpty(taskComputeGroup)) {
- ctx.setCloudCluster(taskComputeGroup);
+ if (!Config.isCloudMode()) {
+ return;
+ }
Review Comment:
Handle it in ctx.setCloudCluster to avoid scattered isCloudMode checks
throughout the code.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -405,10 +409,37 @@ private void installTaskSnapshots(StatementContext
statementContext) {
}
private void setComputeGroup(ConnectContext ctx) {
- String taskComputeGroup = taskContext.getComputeGroup();
- if (Config.isCloudMode() && !Strings.isNullOrEmpty(taskComputeGroup)) {
- ctx.setCloudCluster(taskComputeGroup);
+ if (!Config.isCloudMode()) {
+ return;
+ }
+ // A compute group declared on the MV pins every refresh, automatic or
manual, to that group.
+ // Only when the MV declares nothing does a manual REFRESH keep
borrowing the session's group,
+ // which is the behaviour every existing MV keeps.
+ String declared = mtmv == null ? null :
mtmv.getComputeGroup().orElse(null);
+ String effective = Strings.isNullOrEmpty(declared) ?
taskContext.getComputeGroup() : declared;
+ if (!Strings.isNullOrEmpty(effective)) {
+ ctx.setCloudCluster(effective);
+ }
+ }
+
+ /**
+ * Re-checks the binding before the refresh runs: the groups can be
dropped and privileges
+ * revoked while the MV exists, and without this the refresh would fail
later with an unrelated
+ * message.
+ *
+ * <p>The identity used here is whatever the refresh actually runs as,
which today is the
+ * hardcoded {@code admin} (see {@link
MTMVPlanUtil#createBasicMvContext}). That makes the
+ * privilege half of the check always pass; the existence half is what has
teeth right now. Once
+ * an MV carries a real owner, passing that owner here is the only change
needed.
+ */
+ private void checkBindingBeforeTask(ConnectContext ctx) throws
UserException {
Review Comment:
The function name needs to be reconsidered; "binding" has an expanded scope
and should be limited to the 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]
