[raghav-reglobe]

Background

In the rewrite stage Nereids infers IS NOT NULL predicates so that later rules can turn an outer join into an inner join and prune data. InferAggNotNull infers them from the arguments of the null-ignoring aggregate functions of an aggregate with no GROUP BY, and InferFilterNotNull from a filter's own conjuncts; both mark what they add as generated. The top-down rewrite job applies the rules of a batch to a plan node until none of them changes it.

What problem does this PR solve?

Issue Number: close #70553
Related PR: #33598 (added EliminateAggCaseWhen), #43546 (added TopDownVisitorRewriteJob)
PR Type: Bugfix

Problem Summary:

A statement such as SELECT AVG(CASE WHEN a IS NOT NULL THEN DATEDIFF(a, b) END) FROM t never finishes planning. The FE thread spins in the rewrite stage, the plan grows by one LogicalFilter per pass until the heap is full, and neither query_timeout nor KILL ends it; on our cluster it stopped the master FE. InferAggNotNull read only the filter directly under the aggregate while putting what it generated into a new filter on top, so two not-nulls took turns being "missing", and the visitor rewrite job had no check inside its loop. The rule now reads every filter in the chain under the aggregate, and the job checks the planning timeout on every pass and stops after 10000 rewrites at one position. After an upgrade these statements plan and return; plans of statements that planned before are unchanged in the cases I compared (see Testing), except for the order of one filter's conjuncts in the case described under Behavior change.

Details

• Frequency: Always, for the shape. EliminateAggCaseWhen needs an aggregate with no GROUP BY, exactly one aggregate function and one output expression, a null-ignoring function with one argument, and that argument an IF(cond, x, NULL) (a CASE WHEN cond THEN x END with no ELSE). The loop then needs the filter directly under the aggregate to hold some, but not all, of the IS NOT NULL predicates that the aggregate's argument implies, literally as conjuncts. a IS NOT NULL AND b > 0 still counts as not holding b IS NOT NULL, and with a nested CASE/IF the inner condition is the filter directly under the aggregate.
• Root cause: EliminateAggCaseWhen rewrites avg(if(a IS NOT NULL, f(a, b), NULL)) to avg(f(a, b)) over a new filter a IS NOT NULL. InferAggNotNull infers a IS NOT NULL and b IS NOT NULL, finds a in the child filter and puts b into a new filter on top. On its next pass the child filter is the one holding only b, so a looks missing and gets a filter of its own, then b again, without end. TopDownVisitorRewriteJob.doRewrite runs a node's rules in a while (true) and checked the planning timeout only before that loop.
• Why existing tests missed it: The inputs of InferAggNotNullTest put the aggregate directly over a scan, never over a filter that already holds one of the inferred not-nulls, and no test ran the rewrite stage on a statement of this shape. The rule tests run a rule to a fixpoint, so this shape would have hung a test, not failed it.
• Fix: InferAggNotNull collects the conjuncts of every filter in the chain directly under the aggregate before it decides what is missing, and still puts what it generates into a filter of its own on top. The second pass finds both not-nulls and stops. TopDownVisitorRewriteJob.doRewrite checks the planning timeout at the start of every pass and fails the statement, naming the rule, after 10000 rewrites at one position.
• Other paths and branches: The same InferAggNotNull read and the same visitor loop are on branch-4.0 and branch-4.1 (checked at their heads 8a99617 and af23c8d, and at tags 4.0.6 and 4.1.5), with EliminateAggCaseWhen present on both; the two commits can be picked squashed. The bound is on TopDownVisitorRewriteJob only. The other three rewrite drivers (the stack-based top-down and bottom-up jobs and the bottom-up visitor job) are not changed and have no bound; they check the planning timeout as they go, but only while the statement stopwatch runs, and only NereidsPlanner.plan starts it. A planner entry that does not start it (for example the build of a materialized view's plan cache) has no planning timeout in any driver, as before this PR; there the bound is the only guard and it covers the visitor job alone. With EXPLAIN ... PLAN PROCESS every rewrite also keeps two plan strings, so a rule that grows the plan can fill the heap before it reaches the bound; the comment on the bound says so.
• Behavior change: (1) A rewrite that does not converge on the visitor job now fails the statement with The rewrite does not converge: rule ... still changes ... after 10000 rewrites at one position, or with the planning timeout error, where it used to run until the FE's heap was full. (2) Where two or more filters sit under the aggregate and a lower one already holds an inferred not-null (a nested CASE/IF without ELSE produces this), the rule no longer adds a generated copy of it on top. The predicates, the plan shape and the results are the same; the order of that filter's conjuncts in plain EXPLAIN can differ, for example b > 0 AND a < 10 where it was a < 10 AND b > 0, and with it the filter's row estimate in edge cases. EXPLAIN SHAPE PLAN, which the manual comparison below used, sorts conjuncts and does not show it.

Testing

Unit tests, run on a build machine of ours on this branch's head (master at ccfd437 plus the two commits): the FE builds, mvn checkstyle:check on fe-core passes, and 28 test classes pass with 252 tests, 0 failures, 0 errors. They are the classes below and their neighbours (EliminateAggCaseWhenTest, InferFilterNotNullTest, InferJoinNotNullTest, EliminateOuterJoinTest, EliminateNotNullTest, RewriteTopDownJobTest) plus 20 classes that run the whole rewrite stage.

• InferAggNotNullTest.testInferPutsItsNotNullAboveTheFilterThatHoldsTheOther applies the rule one pass at a time: the first pass puts the missing not-null into a filter on top, the second changes nothing. On master the second pass generates again, so this is the test that fails before the fix (by reading; I did not run it against master).
• InferAggNotNullTest.testFilterUnderTheAggregateStillInfersItsOwnNotNulls: with InferFilterNotNull in the same batch, the filter under the aggregate still gets the not-nulls of its own condition.
• TopDownVisitorRewriteJobTest: a rule that changes its node on every pass is stopped with its name in the error, and the test fails on its assertion if the bound is removed; two positions rewritten 6000 times each do not trip the bound; a rule whose first pass outlasts the planning timeout is stopped before its second pass.

Manual test, on three single-node clusters side by side (FE and BE in one pod, the same four small tables with NULL combinations, the same statistics). They are builds of a master snapshot of early November (1503e39) with unrelated patches: (a) without this PR, (b) with the first commit only, (c) with both commits. 26 statements, each planned with EXPLAIN SHAPE PLAN and executed, with INFER_AGG_NOT_NULL enabled and with it disabled for the session:

• The seven shapes that loop (AVG, SUM and MAX of CASE WHEN x IS NOT NULL THEN f(x, y) END on DATE and on INT columns, the IF form, with a WHERE added, three columns in the branch) return on (b) and (c). They were not sent to (a) in this run; (a) hung on these shapes with AVG and SUM in the run behind the issue.
• On the other 19 statements, (c) plans byte for byte as (a). (b) did not on five of them: SUM(CASE WHEN t1.x + t3.y > 0 THEN t1.z END) over t2 LEFT JOIN t3 ... JOIN t1, SUM and COUNT of CASE WHEN t2.a + t3.b > 0 THEN t1.z END over t1 LEFT JOIN t2 LEFT JOIN t3, the SUM form of that inside a subquery, and SUM(CASE WHEN c + d > 0 THEN k END) ... WHERE c IS NULL. (a) and (c) plan two inner joins, or an empty relation for the last; (b) kept the lower join an outer join, and scanned the table for the last.
• Every statement returns the same value on all three clusters, with the rule enabled and with it disabled.

Production, on our own cluster (storage-compute separated, 3 FE and 8 BE, the same build line as above): the first commit ran there for six hours on 2026-11-13 and both commits run there since 11:59 UTC that day. The statement that had stopped the master FE is a real one, an AVG over a CASE on a table of 1.35 million rows from a nightly check of several hundred KPI queries; with the rule enabled it returns the same value in about 0.6 s on each of the three FEs. Under the first commit, of 2,806 query texts that ran in the same hours on the day before and on that day, one was slower by more than 2x and 1 s and one was faster by the same margin. That comparison has not been repeated for the second commit yet.

There is no regression suite in this PR. I can add one if you want it.

Notes for Reviewers

The first commit ended the loop by adding the generated not-nulls to the filter already under the aggregate. A review found that InferFilterNotNull, which runs next in the same batch, skips a filter that holds a generated not-null, so that filter's own conjuncts were no longer inferred from: for SUM(CASE WHEN t2.a + t3.b > 0 THEN t1.z END) over t1 LEFT JOIN t2 LEFT JOIN t3 the lower join stayed an outer join, and a contradiction with a WHERE x IS NULL was no longer folded to an empty relation. The second commit replaces that approach: the filters below the aggregate stay as written. I left both commits so the change of approach is visible; the PR is meant to be squashed.

The bound is on the visitor job only, because that is the loop that had no check at all. I can add the same bound to the other two jobs here or in a follow-up, whichever you prefer.

Release note

Fix a planner loop: a statement with an aggregate that has no GROUP BY over a CASE/IF without ELSE (also nested) whose condition tests some, but not all, of the argument's columns with IS NOT NULL, for example AVG(CASE WHEN x IS NOT NULL THEN f(x, y) END), never finished planning and exhausted the FE's heap.

Check List (For Author)

• Behavior changed: Yes (see Details)
• Does this need documentation: No
• Pre-submission status: Partially verified: upstream CI has not run on this head, and the manual and production runs were on our own build line (a master snapshot of early November with unrelated patches), not on a build of this branch.

AI Disclosure

• Tool: Claude Code (Claude Fable 5.2); the local review ran on Claude Opus 5.5.
• Generated: the code, the tests and this description.
• Human review: The author directed the work and reviewed the description, the review findings and the test results. The code was reviewed by an automated multi-agent review together with the author, who did not read it line by line.

🤖 Generated with Claude Code

[raghav-reglobe]

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#70554
commit: 9bd2d00c09aade2837b9fda938c0dde14b71ce03
base: ccfd4378d424292a363a96142ad795b8c9ace21e
reviewed_at: 2026-11-13T19:28+05:30
reviewer: raghav-reglobe
model: claude-opus-5-5
effort: xhigh
findings: {blocker: 0, major: 0, minor: 4, nit: 5}
rounds: 1
converged: true

Notes for maintainers

• F-06 (observability, behaviour change not declared at the reviewed commit; the PR description now declares it) fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java:55-59 — where a lower filter under the aggregate already holds an inferred not-null (a nested CASE/IF without ELSE), the rule no longer adds a generated copy on top, so one filter's conjunct order in plain EXPLAIN differs from the merge base; predicates, plan shape and results are the same. Checked by reading, not run.
• F-01 fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/TopDownVisitorRewriteJob.java:37-42 — the rewrite bound covers the visitor job only; the other three drivers have none, and a planner entry that never starts the statement stopwatch has no planning timeout either, as before this PR.
• F-04 fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java:131-192 — no regression or end-to-end test runs the issue's statement, and neither new rule test runs on the visitor driver.
• F-07 fe/fe-core/src/test/java/org/apache/doris/nereids/jobs/rewrite/TopDownVisitorRewriteJobTest.java:128-134 — the planning-timeout test depends on wall-clock time (a 1.1 s sleep); a deterministic form exists.
• The same loop is on branch-4.0 and branch-4.1 (F-03); the PR description names both and the two PRs that introduced the two halves.

_{Reviewed locally with the doris-repo-review pipeline. Repository policy may accept this receipt for the matching commit; it is not a human Apache approval.}