morrySnow commented on code in PR #49514:
URL: https://github.com/apache/doris/pull/49514#discussion_r2043426117
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/SummaryProfile.java:
##########
@@ -227,6 +229,9 @@ public class SummaryProfile {
private long parseSqlFinishTime = -1;
@SerializedName(value = "nereidsLockTableFinishTime")
private long nereidsLockTableFinishTime = -1;
+
+ @SerializedName(value = "nereidsCollectTablePartitionTime")
+ private long nereidsCollectTablePartitionTime = -1;
Review Comment:
```suggestion
private long nereidsCollectTablePartitionFinishTime = -1;
```
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -2139,6 +2142,12 @@ public void setEnableLeftZigZag(boolean
enableLeftZigZag) {
"Whether enable cost based rewrite for sync mv"})
public boolean enableSyncMvCostBasedRewrite = true;
+ @VariableMgr.VarAttr(name = MATERIALIZED_VIEW_REWRITE_DURATION_THRESHOLD,
needForward = true,
Review Comment:
add time unit to variable name, like: `AUTO_PROFILE_THRESHOLD_MS`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/ExpressionLineageReplacer.java:
##########
@@ -63,26 +64,8 @@ public Expression visit(Plan plan, ExpressionReplaceContext
context) {
@Override
public Expression visitGroupPlan(GroupPlan groupPlan,
ExpressionReplaceContext context) {
- Group group = groupPlan.getGroup();
- if (group == null) {
- return visit(groupPlan, context);
- }
- Collection<StructInfo> structInfos =
group.getstructInfoMap().getStructInfos();
- if (structInfos.isEmpty()) {
- return visit(groupPlan, context);
- }
- // Find first info which the context's bitmap contains all to make
sure that
- // the expression lineage is correct
- Optional<StructInfo> structInfoOptional = structInfos.stream()
- .filter(info -> (context.getTableBitSet().isEmpty()
- || StructInfo.containsAll(context.getTableBitSet(),
info.getTableBitSet()))
- && !info.getNamedExprIdAndExprMapping().isEmpty())
- .findFirst();
- if (!structInfoOptional.isPresent()) {
- return visit(groupPlan, context);
- }
-
context.getExprIdExpressionMap().putAll(structInfoOptional.get().getNamedExprIdAndExprMapping());
- return null;
+ LOG.error("ExpressionLineageReplacer should not meet groupPlan, plan
is {}", groupPlan.treeString());
+ return visit(groupPlan, context);
Review Comment:
why not return null directly?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java:
##########
@@ -274,43 +292,51 @@ protected List<Plan> doRewrite(StructInfo
queryStructInfo, CascadesContext casca
continue;
}
Pair<Map<BaseTableInfo, Set<String>>, Map<BaseTableInfo,
Set<String>>> invalidPartitions;
- if (materializationContext instanceof AsyncMaterializationContext)
{
+ if (PartitionCompensator.needUnionRewrite(materializationContext))
{
+ MTMV mtmv = ((AsyncMaterializationContext)
materializationContext).getMtmv();
+ BaseTableInfo relatedTableInfo =
mtmv.getMvPartitionInfo().getRelatedTableInfo();
try {
- invalidPartitions = calcInvalidPartitions(queryPlan,
rewrittenPlan,
- (AsyncMaterializationContext)
materializationContext, cascadesContext);
+ Set<String> queryUsedPartition =
PartitionCompensator.getQueryTableUsedPartition(
+ relatedTableInfo.toList(), queryStructInfo,
cascadesContext.getStatementContext());
+ if (queryUsedPartition.isEmpty()) {
+
materializationContext.recordFailReason(queryStructInfo,
+ String.format("queryUsedPartition is empty,
table is %s, sql hash is %s",
+ relatedTableInfo.toList(),
cascadesContext.getConnectContext().getSqlHash()),
+ () -> String.format("queryUsedPartition is
empty, table is %s, sql hash is %s",
+ relatedTableInfo.toList(),
cascadesContext.getConnectContext().getSqlHash()));
+ LOG.warn(String.format("queryUsedPartition is empty,
table is %s, sql hash is %s",
Review Comment:
why query partition is empty require a warning log? this should not happen?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java:
##########
@@ -114,11 +107,22 @@ public abstract class AbstractMaterializedViewRule
implements ExplorationRuleFac
*/
public List<Plan> rewrite(Plan queryPlan, CascadesContext cascadesContext)
{
List<Plan> rewrittenPlans = new ArrayList<>();
+ SessionVariable sessionVariable =
cascadesContext.getConnectContext().getSessionVariable();
// if available materialization list is empty, bail out
+ StatementContext statementContext =
cascadesContext.getStatementContext();
if (cascadesContext.getMaterializationContexts().isEmpty()) {
return rewrittenPlans;
}
+ if (statementContext.getMaterializedViewRewriteDuration()
+ > sessionVariable.materializedViewRewriteDurationThreshold) {
+ LOG.warn("materialized view rewrite duration is exceeded, the
query sql hash is {}",
+ cascadesContext.getConnectContext().getSqlHash());
+ MaterializationContext.makeFailWithDurationExceeded(queryPlan,
+ cascadesContext.getMaterializationContexts());
+ return rewrittenPlans;
+ }
for (MaterializationContext context :
cascadesContext.getMaterializationContexts()) {
+ statementContext.getMaterializedViewStopwatch().reset().start();
Review Comment:
nit: StopWatch could record the total time used, if u only use start and
stop, but not reset it. so way not use StopWatch only?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java:
##########
@@ -273,43 +291,51 @@ protected List<Plan> doRewrite(StructInfo
queryStructInfo, CascadesContext casca
continue;
}
Pair<Map<BaseTableInfo, Set<String>>, Map<BaseTableInfo,
Set<String>>> invalidPartitions;
- if (materializationContext instanceof AsyncMaterializationContext)
{
+ if (PartitionCompensator.needUnionRewrite(materializationContext))
{
+ MTMV mtmv = ((AsyncMaterializationContext)
materializationContext).getMtmv();
+ BaseTableInfo relatedTableInfo =
mtmv.getMvPartitionInfo().getRelatedTableInfo();
try {
- invalidPartitions = calcInvalidPartitions(queryPlan,
rewrittenPlan,
- (AsyncMaterializationContext)
materializationContext, cascadesContext);
+ Set<String> queryUsedPartition =
PartitionCompensator.getQueryTableUsedPartition(
+ relatedTableInfo.toList(), queryStructInfo,
cascadesContext);
+ if (queryUsedPartition.isEmpty()) {
+
materializationContext.recordFailReason(queryStructInfo,
+ String.format("queryUsedPartition is empty,
table is %s, sql hash is %s",
+ relatedTableInfo.toList(),
cascadesContext.getConnectContext().getSqlHash()),
+ () -> String.format("queryUsedPartition is
empty, table is %s, sql hash is %s",
+ relatedTableInfo.toList(),
cascadesContext.getConnectContext().getSqlHash()));
+ LOG.warn(String.format("queryUsedPartition is empty,
table is %s, sql hash is %s",
+ relatedTableInfo.toList(),
cascadesContext.getConnectContext().getSqlHash()));
+ continue;
+ }
+ invalidPartitions =
calcInvalidPartitions(queryUsedPartition, rewrittenPlan,
+ cascadesContext, (AsyncMaterializationContext)
materializationContext);
} catch (AnalysisException e) {
materializationContext.recordFailReason(queryStructInfo,
"Calc invalid partitions fail",
() -> String.format("Calc invalid partitions fail,
mv partition names are %s",
- ((AsyncMaterializationContext)
materializationContext).getMtmv().getPartitions()));
+ mtmv.getPartitions()));
LOG.warn("Calc invalid partitions fail", e);
continue;
}
if (invalidPartitions == null) {
// if mv can not offer any partition for query, query
rewrite bail out to avoid cycle run
materializationContext.recordFailReason(queryStructInfo,
"mv can not offer any partition for query",
- () -> String.format("mv partition info %s",
- ((AsyncMaterializationContext)
materializationContext).getMtmv()
- .getMvPartitionInfo()));
+ () -> String.format("mv partition info %s",
mtmv.getMvPartitionInfo()));
return rewriteResults;
}
- boolean partitionNeedUnion =
needUnionRewrite(invalidPartitions, cascadesContext);
- boolean canUnionRewrite = canUnionRewrite(queryPlan,
- ((AsyncMaterializationContext)
materializationContext).getMtmv(),
- cascadesContext);
+ boolean partitionNeedUnion =
PartitionCompensator.needUnionRewrite(invalidPartitions, cascadesContext);
+ boolean canUnionRewrite = canUnionRewrite(queryPlan, mtmv,
cascadesContext);
if (partitionNeedUnion && !canUnionRewrite) {
materializationContext.recordFailReason(queryStructInfo,
"need compensate union all, but can not, because
the query structInfo",
() -> String.format("mv partition info is %s, and
the query plan is %s",
- ((AsyncMaterializationContext)
materializationContext).getMtmv()
- .getMvPartitionInfo(),
queryPlan.treeString()));
+ mtmv.getMvPartitionInfo(),
queryPlan.treeString()));
Review Comment:
has updated?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/ExpressionLineageReplacer.java:
##########
@@ -63,26 +64,8 @@ public Expression visit(Plan plan, ExpressionReplaceContext
context) {
@Override
public Expression visitGroupPlan(GroupPlan groupPlan,
ExpressionReplaceContext context) {
- Group group = groupPlan.getGroup();
- if (group == null) {
- return visit(groupPlan, context);
- }
- Collection<StructInfo> structInfos =
group.getstructInfoMap().getStructInfos();
- if (structInfos.isEmpty()) {
- return visit(groupPlan, context);
- }
- // Find first info which the context's bitmap contains all to make
sure that
- // the expression lineage is correct
- Optional<StructInfo> structInfoOptional = structInfos.stream()
- .filter(info -> (context.getTableBitSet().isEmpty()
- || StructInfo.containsAll(context.getTableBitSet(),
info.getTableBitSet()))
- && !info.getNamedExprIdAndExprMapping().isEmpty())
- .findFirst();
- if (!structInfoOptional.isPresent()) {
- return visit(groupPlan, context);
- }
-
context.getExprIdExpressionMap().putAll(structInfoOptional.get().getNamedExprIdAndExprMapping());
- return null;
+ LOG.error("ExpressionLineageReplacer should not meet groupPlan, plan
is {}", groupPlan.treeString());
Review Comment:
use groupPlan.toString here to avoid print too many logs
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/SummaryProfile.java:
##########
@@ -788,8 +799,13 @@ public String getPrettyNereidsRewriteTime() {
return getPrettyTime(nereidsRewriteFinishTime,
nereidsAnalysisFinishTime, TUnit.TIME_MS);
}
+
+ public String getPrettyNereidsCollectTablePartitionTime() {
+ return getPrettyTime(nereidsCollectTablePartitionTime,
nereidsRewriteFinishTime, TUnit.TIME_MS);
Review Comment:
what's mean about this timer?
--
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
