Skip to content

Commit f3c94ed

Browse files
authored
[Fix](nereids) fix topn lazy materialize bug (#55623)
### What problem does this PR solve? 1. RecomputeLogicalPropertiesProcessor must be applied before LazyMaterializeTopN because topn.output may be in wrong order, and hence makes materialize node output slot in wrong order. 2. LazyMaterializeTopN rule only concerns limit, but not offset
1 parent 75bc6d8 commit f3c94ed

File tree

3 files changed

+139
-3
lines changed

3 files changed

+139
-3
lines changed

fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PlanPostProcessors.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,17 @@ public List<PlanPostProcessor> getProcessors() {
6565
Builder<PlanPostProcessor> builder = ImmutableList.builder();
6666
builder.add(new PushDownFilterThroughProject());
6767
builder.add(new RemoveUselessProjectPostProcessor());
68+
builder.add(new RecomputeLogicalPropertiesProcessor());
69+
/*
70+
1. LazyMaterializeTopN should be applied before MergeProjectPostProcessor
71+
2. LazyMaterializeTopN should be applied after RecomputeLogicalPropertiesProcessor
72+
PhysicalLazyMaterialize.materializedSlots should be subsequence of topN.getOutput().
73+
*/
6874
if (cascadesContext.getConnectContext().getSessionVariable().enableTopnLazyMaterialization()) {
69-
// LazyMaterializeTopN should run before MergeProjectPostProcessor
7075
builder.add(new LazyMaterializeTopN());
7176
}
7277
builder.add(new MergeProjectPostProcessor());
73-
builder.add(new RecomputeLogicalPropertiesProcessor());
78+
7479
if (cascadesContext.getConnectContext().getSessionVariable().enableAggregateCse) {
7580
builder.add(new ProjectAggregateExpressionsForCse());
7681
}

fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/LazyMaterializeTopN.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public Plan visitPhysicalTopN(PhysicalTopN topN, CascadesContext ctx) {
6767
if (hasMaterialized) {
6868
return topN;
6969
}
70-
if (SessionVariable.getTopNLazyMaterializationThreshold() < topN.getLimit() + topN.getOffset()) {
70+
if (SessionVariable.getTopNLazyMaterializationThreshold() < topN.getLimit()) {
7171
return topN;
7272
}
7373
/*

0 commit comments

Comments
 (0)