Skip to content

branch-4.2: [fix](nereids) fix limit + offset overflow when pushing down TopN/Limit #64633 #68186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
924060929 wants to merge 1 commit into
base: branch-4.2
Choose a base branch
from
Open

branch-4.2: [fix](nereids) fix limit + offset overflow when pushing down TopN/Limit #64633 #68186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion ...e-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2710,7 +2710,13 @@ public PlanFragment visitPhysicalTopN(PhysicalTopN<? extends Plan> topN, PlanTra
// push sort to scan opt
if (sortNode.getChild(0) instanceof OlapScanNode) {
OlapScanNode scanNode = ((OlapScanNode) sortNode.getChild(0));
if (checkPushSort(sortNode, scanNode.getOlapTable())) {
// When the offset is set, the pushed scan limit is limit + offset. If that overflows the
// long range it would wrap to a negative limit, so skip the whole push-sort-to-scan
// optimization (do not push the sort info either); the sort node still applies the real
// limit/offset. Sort info and sort limit are always pushed together.
boolean limitOverflows = sortNode.getOffset() > 0
&& Utils.addOverflows(sortNode.getLimit(), sortNode.getOffset());
if (checkPushSort(sortNode, scanNode.getOlapTable()) && !limitOverflows) {
SortInfo sortInfo = sortNode.getSortInfo();
scanNode.setSortInfo(sortInfo);
scanNode.getSortInfo().setSortTupleSlotExprs(sortNode.getResolvedTupleExprs());
Expand Down
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PushTopnToAgg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.doris.nereids.trees.plans.physical.PhysicalHashAggregate.TopnPushInfo;
import org.apache.doris.nereids.trees.plans.physical.PhysicalProject;
import org.apache.doris.nereids.trees.plans.physical.PhysicalTopN;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.qe.ConnectContext;

/**
Expand All @@ -49,6 +50,11 @@ public class PushTopnToAgg extends PlanPostProcessor {
@Override
public Plan visitPhysicalTopN(PhysicalTopN<? extends Plan> topN, CascadesContext ctx) {
topN.child().accept(this, ctx);
if (Utils.addOverflows(topN.getLimit(), topN.getOffset())) {
// limit + offset overflows the long range: no aggregate can hold that many rows, so
// pushing a topn limit into the aggregate cannot reduce anything; leave the topn as is.
return topN;
}
if (ConnectContext.get().getSessionVariable().topnOptLimitThreshold <= topN.getLimit() + topN.getOffset()
&& !ConnectContext.get().getSessionVariable().pushTopnToAgg) {
return topN;
Expand Down
15 changes: 12 additions & 3 deletions ...rc/main/java/org/apache/doris/nereids/rules/implementation/LogicalTopNToPhysicalTopN.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@

package org.apache.doris.nereids.rules.implementation;

import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.SortPhase;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.trees.plans.physical.PhysicalTopN;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.Lists;
Expand All @@ -46,13 +48,20 @@ public Rule build() {
* mergeTopN(limit, off, require gather) -> localTopN(off+limit, 0, require any)
*/
private List<PhysicalTopN<? extends Plan>> twoPhaseSort(LogicalTopN<? extends Plan> logicalTopN) {
PhysicalTopN<Plan> localSort = new PhysicalTopN<>(logicalTopN.getOrderKeys(),
logicalTopN.getLimit() + logicalTopN.getOffset(), 0, SortPhase.LOCAL_SORT,
logicalTopN.getLogicalProperties(), logicalTopN.child(0));
int sortPhaseNum = 0;
if (ConnectContext.get() != null) {
sortPhaseNum = ConnectContext.get().getSessionVariable().sortPhaseNum;
}
// The local sort keeps limit + offset rows. When that overflows the long range (e.g. LIMIT
// and OFFSET both BIGINT_MAX), we cannot produce a valid physical TopN: the two-phase
// MERGE_SORT needs a local limit that overflows, and even a single-phase GATHER_SORT would
// pass the overflowing limit/offset to BE where HeapSorter also computes limit + offset.
if (Utils.addOverflows(logicalTopN.getLimit(), logicalTopN.getOffset())) {
throw new AnalysisException("limit + offset overflows the long range");
}
PhysicalTopN<Plan> localSort = new PhysicalTopN<>(logicalTopN.getOrderKeys(),
logicalTopN.getLimit() + logicalTopN.getOffset(), 0, SortPhase.LOCAL_SORT,
logicalTopN.getLogicalProperties(), logicalTopN.child(0));
if (sortPhaseNum == 1) {
PhysicalTopN<Plan> onePhaseSort = new PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
logicalTopN.getOffset(), SortPhase.GATHER_SORT,
Expand Down
13 changes: 11 additions & 2 deletions ...-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CollectLimitAboveConsumer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalCTEConsumer;
import org.apache.doris.nereids.trees.plans.logical.LogicalLimit;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.util.Utils;

import com.google.common.collect.ImmutableList;

Expand Down Expand Up @@ -52,7 +53,15 @@ public List<Rule> buildRules() {

private void collectLimitRows(CascadesContext cascadesContext, LogicalLimit<?> limit,
LogicalCTEConsumer cteConsumer) {
cascadesContext.putConsumerIdToLimitRows(
cteConsumer.getRelationId(), limit.getLimit() + limit.getOffset());
// The recorded value is the number of rows this consumer needs (limit + offset). When that
// overflows the long range the consumer effectively needs all rows, so do not record anything:
// tryToConstructLimit treats a missing entry as "unbounded" and leaves the producer unlimited
// (this consumer's own limit still applies above it). Recording a wrapped-around negative count
// would corrupt the producer row bound.
if (Utils.addOverflows(limit.getLimit(), limit.getOffset())) {
return;
}
cascadesContext.putConsumerIdToLimitRows(cteConsumer.getRelationId(),
limit.getLimit() + limit.getOffset());
}
}
5 changes: 5 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/LimitAggToTopNAgg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -57,6 +58,7 @@ public List<Rule> buildRules() {
logicalLimit(logicalAggregate())
.when(limit -> ConnectContext.get() != null
&& ConnectContext.get().getSessionVariable().pushTopnToAgg
&& !Utils.addOverflows(limit.getLimit(), limit.getOffset())
&& ConnectContext.get().getSessionVariable().topnOptLimitThreshold
>= limit.getLimit() + limit.getOffset())
.when(limit -> {
Expand All @@ -73,6 +75,7 @@ public List<Rule> buildRules() {
logicalLimit(logicalProject(logicalAggregate()))
.when(limit -> ConnectContext.get() != null
&& ConnectContext.get().getSessionVariable().pushTopnToAgg
&& !Utils.addOverflows(limit.getLimit(), limit.getOffset())
&& ConnectContext.get().getSessionVariable().topnOptLimitThreshold
>= limit.getLimit() + limit.getOffset())
.when(limit -> {
Expand All @@ -93,6 +96,7 @@ public List<Rule> buildRules() {
logicalTopN(logicalAggregate())
.when(topn -> ConnectContext.get() != null
&& ConnectContext.get().getSessionVariable().pushTopnToAgg
&& !Utils.addOverflows(topn.getLimit(), topn.getOffset())
&& ConnectContext.get().getSessionVariable().topnOptLimitThreshold
>= topn.getLimit() + topn.getOffset())
.when(topn -> {
Expand All @@ -115,6 +119,7 @@ public List<Rule> buildRules() {
logicalTopN(logicalProject(logicalAggregate()))
.when(topn -> ConnectContext.get() != null
&& ConnectContext.get().getSessionVariable().pushTopnToAgg
&& !Utils.addOverflows(topn.getLimit(), topn.getOffset())
&& ConnectContext.get().getSessionVariable().topnOptLimitThreshold
>= topn.getLimit() + topn.getOffset())
.when(topn -> {
Expand Down
11 changes: 11 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeLimits.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@

package org.apache.doris.nereids.rules.rewrite;

import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalLimit;
import org.apache.doris.nereids.util.Utils;

/**
* This rule aims to merge consecutive limits.
Expand Down Expand Up @@ -54,7 +56,16 @@ public Rule build() {
}).toRule(RuleType.MERGE_LIMITS);
}

/**
* Merge two consecutive limits' offsets into a single offset. Consecutive limits must be merged,
* and an overflowing combined offset cannot be represented as a single offset, so fail fast
* instead of wrapping to a negative offset.
*/
public static long mergeOffset(long upperOffset, long bottomOffset) {

@github-actions github-actions Bot Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve valid nested LIMITs when only the merged offset overflows

Two nested LIMIT 1 OFFSET 9223372036854775806 nodes are individually representable because each limit + offset equals Long.MAX_VALUE, and the outer node necessarily returns zero rows because its offset exceeds the child's one-row output. This guard nevertheless turns that valid query into an analysis error even though mergeLimit already derives a zero limit. Please canonicalize this case to a representable empty limit; PhysicalPlanTranslator's Exchange fold at lines 2263-2265 needs the same zero-limit handling so it does not call mergeOffset after deriving zero.

if (Utils.addOverflows(upperOffset, bottomOffset)) {
throw new AnalysisException(
"offset overflows long range when merging limits: " + upperOffset + " + " + bottomOffset);
}
return upperOffset + bottomOffset;
}

Expand Down
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeTopNs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

package org.apache.doris.nereids.rules.rewrite;

import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.properties.OrderKey;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.util.Utils;

import java.util.List;

Expand Down Expand Up @@ -58,6 +60,10 @@ public Rule build() {
long limit = topN.getLimit();
long childOffset = childTopN.getOffset();
long childLimit = childTopN.getLimit();

@github-actions github-actions Bot Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Keep individually valid nested TopNs executable

Compatible nested TopNs can hit this guard even when each node is valid. For example, two TopNs with limit 1 and offset 9223372036854775806 each have limit + offset == Long.MAX_VALUE; the outer one must return zero rows, but their combined offsets overflow and this code throws before lines 68-73 derive newLimit = 0. The ordered derived-table shape is already known to reach MERGE_TOP_N in test_merge_topn_offset.groovy. Please canonicalize the empty result with a representable offset before this check, and add a boundary case for it.

if (Utils.addOverflows(offset, childOffset)) {
throw new AnalysisException(
"offset overflows long range when merging TopNs: " + offset + " + " + childOffset);
}
long newOffset = offset + childOffset;
// The parent's offset is applied on top of the child's output, so only
// (childLimit - offset) of the child's rows survive. Clamp the merged limit
Expand Down
8 changes: 8 additions & 0 deletions ...rc/main/java/org/apache/doris/nereids/rules/rewrite/PushDownLimitDistinctThroughJoin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
import org.apache.doris.nereids.trees.plans.logical.LogicalLimit;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.ImmutableList;
Expand All @@ -46,6 +47,7 @@ public List<Rule> buildRules() {
.when(LogicalAggregate::isDistinct))
.when(limit ->
ConnectContext.get() != null
&& !Utils.addOverflows(limit.getLimit(), limit.getOffset())
&& ConnectContext.get().getSessionVariable().topnOptLimitThreshold
>= limit.getLimit() + limit.getOffset())
.then(limit -> {
Expand All @@ -64,6 +66,12 @@ public List<Rule> buildRules() {
logicalLimit(logicalAggregate(logicalProject(logicalJoin()).when(LogicalProject::isAllSlots))
.when(LogicalAggregate::isDistinct))
.then(limit -> {
// limit + offset overflowing means no child can hold that many rows, so the
// push-down cannot reduce anything; skip it. (The direct branch is gated the
// same way via topn_opt_limit_threshold.)
if (Utils.addOverflows(limit.getLimit(), limit.getOffset())) {
return null;
}
LogicalAggregate<LogicalProject<LogicalJoin<Plan, Plan>>> agg = limit.child();
LogicalProject<LogicalJoin<Plan, Plan>> project = agg.child();
LogicalJoin<Plan, Plan> join = project.child();
Expand Down
7 changes: 7 additions & 0 deletions ...c/main/java/org/apache/doris/nereids/rules/rewrite/PushDownLimitDistinctThroughUnion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalLimit;
import org.apache.doris.nereids.trees.plans.logical.LogicalUnion;
import org.apache.doris.nereids.util.ExpressionUtils;
import org.apache.doris.nereids.util.Utils;

import com.google.common.collect.ImmutableList;

Expand Down Expand Up @@ -64,6 +65,12 @@ public List<Rule> buildRules() {
logicalLimit(logicalAggregate(logicalUnion().when(union -> union.getQualifier() == Qualifier.ALL))
.when(agg -> agg.isDistinct()))
.then(limit -> {
// limit + offset overflowing the long range means no child can hold that
// many rows, so pushing the limit below the union cannot reduce anything;
// skip the rewrite. The parent limit still applies the original limit/offset.
if (Utils.addOverflows(limit.getLimit(), limit.getOffset())) {
return null;
}
LogicalAggregate<LogicalUnion> agg = limit.child();
LogicalUnion union = agg.child();

Expand Down
8 changes: 4 additions & 4 deletions ...e/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.util.ExpressionUtils;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.thrift.TExprOpcode;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -189,10 +190,9 @@ private Plan pushDown(
}

// When limit + offset overflows the long range, the pushed scan limit would wrap to a
// negative value. Fail with the same error as ordinary TopN instead of leaving score()
// unmaterialized and reporting an unrelated score() usage error.
if (topN.getLimit() > Long.MAX_VALUE - topN.getOffset()) {
throw new AnalysisException("limit + offset overflows the long range");
// negative value; skip the push-down and let the TopN above the scan apply limit/offset.

@github-actions github-actions Bot Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve the overflow error for score() queries

Returning null here leaves score() unmaterialized. The same rewrite batch immediately runs CheckScoreUsage, so a query that has MATCH, ORDER BY, and LIMIT now fails with the unrelated "score() function requires ..." message before LogicalTopNToPhysicalTopN can report the overflow. This also contradicts the existing regression at test_search_score_topn_predicates.groovy:191-201, and the base version of this hunk explicitly threw the standard overflow error to avoid exactly this path. Please keep the fail-fast AnalysisException here (or validate overflow before CheckScoreUsage).

if (Utils.addOverflows(topN.getLimit(), topN.getOffset())) {
return null;
}

long scoreLimit = topN.getLimit() + topN.getOffset();
Expand Down
18 changes: 14 additions & 4 deletions ...src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNDistinctThroughJoin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.util.PlanUtils;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.qe.ConnectContext;

import com.google.common.collect.ImmutableList;
Expand All @@ -49,6 +50,7 @@ public List<Rule> buildRules() {
// TODO: complex order by
.when(topn ->
ConnectContext.get() != null
&& !Utils.addOverflows(topn.getLimit(), topn.getOffset())
&& ConnectContext.get().getSessionVariable().topnOptLimitThreshold
>= topn.getLimit() + topn.getOffset())
.when(topN -> topN.getOrderKeys().stream().map(OrderKey::getExpr)
Expand All @@ -70,6 +72,13 @@ public List<Rule> buildRules() {
.when(topN -> topN.getOrderKeys().stream().map(OrderKey::getExpr)
.allMatch(Slot.class::isInstance))
.then(topN -> {
// limit + offset overflowing the long range means no child can hold that
// many rows, so pushing the TopN below the join cannot reduce anything;
// skip the rewrite. (The direct branch is gated the same way via
// topn_opt_limit_threshold.)
if (Utils.addOverflows(topN.getLimit(), topN.getOffset())) {
return null;
}
LogicalAggregate<LogicalProject<LogicalJoin<Plan, Plan>>> distinct = topN.child();
LogicalProject<LogicalJoin<Plan, Plan>> project = distinct.child();
LogicalJoin<Plan, Plan> join = project.child();
Expand All @@ -95,6 +104,7 @@ public List<Rule> buildRules() {
}

private Plan pushTopNThroughJoin(LogicalTopN<? extends Plan> topN, LogicalJoin<Plan, Plan> join) {
long childLimit = topN.getLimit() + topN.getOffset();
Set<Slot> groupBySlots = ((LogicalAggregate<?>) topN.child()).getGroupByExpressions().stream()
.flatMap(e -> e.getInputSlots().stream()).collect(Collectors.toSet());
switch (join.getJoinType()) {
Expand All @@ -107,7 +117,7 @@ private Plan pushTopNThroughJoin(LogicalTopN<? extends Plan> topN, LogicalJoin<P
join.left().getOutputSet(), topN.getOrderKeys());
if (!pushedOrderKeys.isEmpty()) {
LogicalTopN<Plan> left = topN.withLimitOrderKeyAndChild(
topN.getLimit() + topN.getOffset(), 0, pushedOrderKeys,
childLimit, 0, pushedOrderKeys,
PlanUtils.distinct(join.left()));
return join.withChildren(left, join.right());
}
Expand All @@ -122,7 +132,7 @@ private Plan pushTopNThroughJoin(LogicalTopN<? extends Plan> topN, LogicalJoin<P
join.right().getOutputSet(), topN.getOrderKeys());
if (!pushedOrderKeys.isEmpty()) {
LogicalTopN<Plan> right = topN.withLimitOrderKeyAndChild(
topN.getLimit() + topN.getOffset(), 0, pushedOrderKeys,
childLimit, 0, pushedOrderKeys,
PlanUtils.distinct(join.right()));
return join.withChildren(join.left(), right);
}
Expand All @@ -135,14 +145,14 @@ private Plan pushTopNThroughJoin(LogicalTopN<? extends Plan> topN, LogicalJoin<P
join.left().getOutputSet(), topN.getOrderKeys());
if (!(join.left() instanceof TopN) && !leftPushedOrderKeys.isEmpty()) {
leftChild = topN.withLimitOrderKeyAndChild(
topN.getLimit() + topN.getOffset(), 0, leftPushedOrderKeys,
childLimit, 0, leftPushedOrderKeys,
PlanUtils.distinct(join.left()));
}
List<OrderKey> rightPushedOrderKeys = getPushedOrderKeys(groupBySlots,
join.right().getOutputSet(), topN.getOrderKeys());
if (!(join.right() instanceof TopN) && !rightPushedOrderKeys.isEmpty()) {
rightChild = topN.withLimitOrderKeyAndChild(
topN.getLimit() + topN.getOffset(), 0, rightPushedOrderKeys,
childLimit, 0, rightPushedOrderKeys,
PlanUtils.distinct(join.right()));
}
if (leftChild == join.left() && rightChild == join.right()) {
Expand Down
Loading
Loading