github-actions[bot] commented on code in PR #67532:
URL: https://github.com/apache/doris/pull/67532#discussion_r3944121372
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/SummaryProfile.java:
##########
@@ -1046,11 +1046,19 @@ public String
getPrettyNereidsCollectTablePartitionTime() {
}
public String getPrettyNereidsPreRewriteByMvTime() {
- return getPrettyTime(nereidsPreRewriteByMvFinishTime,
nereidsCollectTablePartitionFinishTime, TUnit.TIME_MS);
+ long start = nereidsCollectTablePartitionFinishTime != -1
+ ? nereidsCollectTablePartitionFinishTime
+ : nereidsRewriteFinishTime;
+ return getPrettyTime(nereidsPreRewriteByMvFinishTime, start,
TUnit.TIME_MS);
}
public String getPrettyNereidsOptimizeTime() {
- return getPrettyTime(nereidsOptimizeFinishTime,
nereidsPreRewriteByMvFinishTime, TUnit.TIME_MS);
+ long start = nereidsPreRewriteByMvFinishTime != -1
+ ? nereidsPreRewriteByMvFinishTime
+ : (nereidsCollectTablePartitionFinishTime != -1
Review Comment:
[P2] Distinguish a skipped pre-MV phase from an attempted one
A missing `nereidsPreRewriteByMvFinishTime` does not only mean that this
phase was skipped. When `needPreMvRewrite` is true,
`PreMaterializedViewRewriter.rewrite()` can run its `Optimizer` and still
return `null` because no MV is chosen (and caught failures follow the same
path); `preMaterializedViewRewrite()` then returns on an empty result before
setting the finish marker. This fallback starts at collection time, so it
reports the whole discarded pre-MV attempt—including a near-timeout attempt—as
ordinary `Optimize Time` while `Pre Rewrite By MV Time` remains `N/A`. Please
record the phase finish whenever the phase is actually entered, reserve this
fallback for a true skip, and test the attempted-but-empty case.
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/SummaryProfile.java:
##########
@@ -1046,11 +1046,19 @@ public String
getPrettyNereidsCollectTablePartitionTime() {
}
public String getPrettyNereidsPreRewriteByMvTime() {
- return getPrettyTime(nereidsPreRewriteByMvFinishTime,
nereidsCollectTablePartitionFinishTime, TUnit.TIME_MS);
+ long start = nereidsCollectTablePartitionFinishTime != -1
+ ? nereidsCollectTablePartitionFinishTime
+ : nereidsRewriteFinishTime;
+ return getPrettyTime(nereidsPreRewriteByMvFinishTime, start,
TUnit.TIME_MS);
}
public String getPrettyNereidsOptimizeTime() {
- return getPrettyTime(nereidsOptimizeFinishTime,
nereidsPreRewriteByMvFinishTime, TUnit.TIME_MS);
+ long start = nereidsPreRewriteByMvFinishTime != -1
+ ? nereidsPreRewriteByMvFinishTime
+ : (nereidsCollectTablePartitionFinishTime != -1
+ ? nereidsCollectTablePartitionFinishTime
+ : nereidsRewriteFinishTime);
+ return getPrettyTime(nereidsOptimizeFinishTime, start, TUnit.TIME_MS);
Review Comment:
[P2] Keep numeric optimize telemetry valid on this skip path
When MV rewrite is disabled, no `InitMaterializationContextHook` records
`nereidsCollectTablePartitionFinishTime`, while `rewrite()` and `optimize()`
still record their finish markers. This getter now correctly renders
`optimizeFinish - rewriteFinish`, but `getNereidsOptimizeTimeMs()` still
computes against the missing collection marker and returns `-1`. As a result
`StmtExecutor` omits the `plan_optimize_duration` sample and completed-plan
audit data still contains `"optimize":-1`. Please preserve the numeric getter's
existing aggregate boundary by using collection when it exists and falling back
to rewrite only when collection is absent, then cover this no-hook case.
--
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]
