raghav-reglobe opened a new issue, #70553: URL: https://github.com/apache/doris/issues/70553
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues. ### Version A build of master at 1503e390275c (the FE reports `doris-4.1.0`) with unrelated patches on top. The three files involved (`InferAggNotNull.java`, `EliminateAggCaseWhen.java`, `TopDownVisitorRewriteJob.java`) have not changed on master since September (their last commits are 2f6358db28d, 19a6089e4f9 and 9d576e9e00a), so I expect current master to behave the same. I have not run a build of it. ### What's Wrong? A statement like this never returns: ```sql SELECT AVG(CASE WHEN a IS NOT NULL THEN DATEDIFF(a, b) END) FROM t; ``` The FE thread that plans it spins in the rewrite stage. The plan grows by one `LogicalFilter` on every pass, the heap fills (4 GB in about 30 seconds in my test), and the FE stops answering. `query_timeout` does not end the statement and it cannot be killed. `EXPLAIN` of the statement hangs the same way. The table can be empty. What happens, as I read the code: 1. `EliminateAggCaseWhen` rewrites `avg(if(a IS NOT NULL, datediff(a, b), NULL))` into `avg(datediff(a, b))` over a new filter directly under the aggregate: ``` LogicalAggregate ( groupByExpr=[], outputExpr=[avg(datediff(a#1, b#2)) AS ...] ) +--LogicalFilter ( predicates=( not a#1 IS NULL) ) +--LogicalProject ( projects=[a#1, b#2] ) +--LogicalOlapScan ( qualified=internal.repro.t, ... ) ``` 2. `InferAggNotNull` then infers `a IS NOT NULL` and `b IS NOT NULL` from the aggregate's argument. It looks for them only in the conjuncts of the aggregate's child filter, and puts what is missing into a new filter on top of it (`PlanUtils.filter(needGenerateNotNulls, agg.child())`). `a` is found, so the new filter holds only `b`. On the next pass the child filter holds only `b`, so `a` is missing and gets a filter of its own, then `b` again, and so on. 3. `TopDownVisitorRewriteJob.doRewrite` applies a node's rules in a `while (true)` until the node stops changing. The planning timeout is checked outside that loop, so nothing ends it. Evidence: a thread dump shows `InferAggNotNull.lambda$build$1` under `TopDownVisitorRewriteJob.doRewrite`. A class histogram taken 1.5 s and 3 s into the statement shows 241,054 and then 642,701 live `LogicalFilter` instances, each holding one generated not-null. ### What You Expected? The statement plans and returns. And a rule that does not converge should fail the statement, not take the FE down. ### How to Reproduce? Use a test FE: it has to be restarted after any of the hanging statements. ```sql CREATE DATABASE IF NOT EXISTS repro; CREATE TABLE repro.t ( k INT NOT NULL, a DATE NULL, b DATE NULL, c INT NULL, d INT NULL ) DUPLICATE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES ("replication_num" = "1"); -- each of these never returns SELECT AVG(CASE WHEN a IS NOT NULL THEN DATEDIFF(a, b) END) FROM repro.t; SELECT SUM(CASE WHEN c IS NOT NULL THEN c + d END) FROM repro.t; SELECT AVG(IF(c IS NOT NULL, c + d, NULL)) FROM repro.t; EXPLAIN SELECT AVG(CASE WHEN a IS NOT NULL THEN DATEDIFF(a, b) END) FROM repro.t; -- each of these returns at once SELECT AVG(DATEDIFF(a, b)) FROM repro.t; SELECT AVG(CASE WHEN a IS NOT NULL AND b IS NOT NULL THEN DATEDIFF(a, b) END) FROM repro.t; SELECT k, AVG(CASE WHEN a IS NOT NULL THEN DATEDIFF(a, b) END) FROM repro.t GROUP BY k; SET disable_nereids_rules = 'INFER_AGG_NOT_NULL'; SELECT AVG(CASE WHEN a IS NOT NULL THEN DATEDIFF(a, b) END) FROM repro.t; ``` The shape that triggers it: no GROUP BY, an aggregate that ignores NULLs, a CASE with no ELSE (or the IF form), and a branch that is also NULL when a column other than the tested one is NULL. ### Anything Else? Workaround: `SET GLOBAL disable_nereids_rules = 'INFER_AGG_NOT_NULL'`. A fix that works for me: - In `InferAggNotNull`, when the aggregate's child is already a filter, add the generated not-nulls to that filter's conjuncts, as `InferFilterNotNull` does, and do not put a second filter on top. - In `TopDownVisitorRewriteJob.doRewrite`, check the planning timeout on every pass, as the bottom-up job does, and fail the statement once one node has been rewritten a bounded number of times. With both, the statements above return, the rule fires once and leaves one filter with both not-nulls, and results are the same with the rule on and off. I can send this as a PR. ### Are you willing to submit PR? - [x] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- 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]
