github-actions[bot] commented on code in PR #63563:
URL: https://github.com/apache/doris/pull/63563#discussion_r3388131227
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -950,6 +950,15 @@ private static List<RewriteJob> getWholeTreeRewriteJobs(
rewriteJobs.addAll(jobs(
topic("rewrite cte sub-tree after sub path
push down",
custom(RuleType.CLEAR_CONTEXT_STATUS, ClearContextStatus::new),
+ // ClearContextStatus wipes
cteIdToOutputIds and consumerIdToFilters;
+ // re-collect them so
RewriteCteChildren can prune producer outputs
+ // and construct push-down filters
correctly. Without this, the
+ // second-pass RewriteCteChildren
sees null maps and either NPEs
+ // (see #57348) or silently skips
the projection optimization.
+ topDown(
Review Comment:
This re-collect job is still built inside the outer
`notTraverseChildrenOf(LogicalCTEAnchor.class)` wrapper used by
`getWholeTreeRewriteJobs`. After `PullUpCteAnchor`, the plan is a chain of
`LogicalCTEAnchor`s, so this `topDown` job stops at the first anchor and does
not visit the consumers under the anchor right side or under nested producers.
`RewriteCteChildren.visitLogicalCTEAnchor` later uses `outer.foreach(...)`,
which *does* traverse those nested anchors/producers and can record consumers
whose `cteIdToOutputIds`/`consumerIdToFilters` were never re-collected. In that
case the second pass still falls back to the null guards and skips the
producer-output pruning/filter construction this PR is intended to restore.
Please run this collection from the CTE child rewrite path, or otherwise use a
traversal that does not stop at `LogicalCTEAnchor`, and add a plan-shape
assertion for the nested CTE case so pruning is covered, not only query success.
##########
regression-test/suites/nereids_rules_p0/cte/test_nested_cte_with_set_op.groovy:
##########
@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Regression test for https://github.com/apache/doris/issues/57348
+// RewriteCteChildren used to NPE on producerOutputs because the
+// second-pass RewriteCteChildren ran after ClearContextStatus wiped
+// cteIdToOutputIds without re-collecting it.
+suite("test_nested_cte_with_set_op") {
+ sql "SET enable_nereids_planner=true"
+ sql "SET enable_fallback_to_original_planner=false"
+
+ sql "drop table if exists test_sales_57348"
+
+ sql """
+ CREATE TABLE `test_sales_57348` (
+ `id` int,
+ `item` varchar(64),
+ `amount` double,
+ `year` int,
+ `month` int
+ ) ENGINE=OLAP
+ DUPLICATE KEY(`id`)
+ DISTRIBUTED BY HASH(`id`) BUCKETS 1
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1"
+ );
+ """
+
+ sql """
+ insert into test_sales_57348 values
+ (1, 'a', 10.0, 2023, 1),
+ (2, 'b', 20.0, 2024, 2),
+ (3, 'c', 30.0, 2025, 3);
+ """
+
+ // The original repro: nested CTEs with EXCEPT plus an unused CTE whose
+ // body contains scalar subqueries over the other CTEs, joined by UNION
ALL.
+ // Before the fix this threw NullPointerException in RewriteCteChildren
during
+ // the second rewrite pass. The bug surfaces at planning time, so a
successful
+ // execution alone proves the fix. We avoid qt_ here to keep the test
+ // self-contained (no .out baseline to maintain for double-precision
results).
+ sql """
+ with
+ tbl1 as (select amount from test_sales_57348 where year = 2023
+ EXCEPT
+ select amount from test_sales_57348 where year = 2024),
+ tbl2 as (select amount from test_sales_57348 where year = 2024
+ EXCEPT
+ select amount from test_sales_57348 where year = 2025),
+ total as (select (select count(1) from tbl1), (select count(1) from
tbl2))
+ select * from tbl1 UNION ALL select * from tbl2
+ order by amount;
+ """
+
+ sql "drop table if exists test_sales_57348"
+}
Review Comment:
Regression tests should drop setup tables before use but leave them after
the suite so a failed run preserves repro state for debugging. This file
already does `drop table if exists test_sales_57348` before creating the table,
so the final drop should be removed.
--
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]
