github-actions[bot] commented on code in PR #66981:
URL: https://github.com/apache/doris/pull/66981#discussion_r3819828536
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CTEInline.java:
##########
@@ -135,4 +139,36 @@ private boolean
containsNondeterministicFunction(LogicalCTEProducer<?> producer)
producer.accept(NondeterministicFunctionCollector.INSTANCE,
nondeterministicFunctions);
return !nondeterministicFunctions.isEmpty();
}
+
+ /**
+ * Return true if the CTE producer's subtree is a LogicalOneRowRelation
and its
+ * final output slots are all compile-time constants.
+ */
+ private static boolean isConstantOneRowProducer(Plan producerRoot) {
+ if (!(producerRoot instanceof LogicalCTEProducer)) {
+ return false;
+ }
+ Plan node = ((LogicalCTEProducer<?>) producerRoot).child();
+ while (node instanceof LogicalSubQueryAlias) {
+ if (node.arity() != 1) {
+ return false;
+ }
+ node = node.child(0);
+ }
+ if (node instanceof LogicalProject) {
+ LogicalProject<?> project = (LogicalProject<?>) node;
+ if (project.arity() != 1) {
+ return false;
+ }
+ if (!ExpressionUtils.allMatch(project.getProjects(),
Expression::isConstant)) {
Review Comment:
[P1] Classify the canonical production producer
Production runs `NORMALIZE_PLAN_JOBS` before this custom rewrite. It changes
a simple producer into `Project(c.a, c.b) -> OneRow`, and the direct-join
example has no `LogicalApply`, so no later `MergeProjectable` removes that
forwarding project. These `SlotReference` projects return false from
`Expression.isConstant()`, leaving the exact positive case materialized.
Resolving only those slots is still insufficient: `SELECT 1 WHERE TRUE` and
`SELECT 1 LIMIT 1` retain `Filter(TRUE)`/`Limit(1)` wrappers here because their
eliminators run only after the inline decision. The tests hide these shapes by
invoking `CTEInline` directly after analysis. Please classify a
canonical/simplified producer and cover the registered production path via
`rewrite()`.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CTEInline.java:
##########
@@ -95,6 +98,7 @@ public Plan visitLogicalCTEAnchor(LogicalCTEAnchor<? extends
Plan, ? extends Pla
LogicalCTEProducer<?> cteProducer = (LogicalCTEProducer<?>)
cteAnchor.left();
if (connectContext.getSessionVariable().enableCTEMaterialize
&& (consumers.size() >
connectContext.getSessionVariable().inlineCTEReferencedThreshold
+ && !isConstantOneRowProducer(cteProducer)
Review Comment:
[P2] Keep runtime-only constant expressions materialized
`Expression.isConstant()` is broader than the compile-time constants
described here. For example, `WITH c AS (SELECT sleep(1) AS s) SELECT (SELECT s
FROM c), (SELECT s FROM c)` reaches a one-row producer after scalar-subquery
project merging; `sleep(1)` is deterministic, so this helper returns true and
the nondeterministic check does not block it. BE constant folding deliberately
skips `sleep`, and materialization evaluates the producer once and multicasts
it, whereas `visitLogicalCTEConsumer` now deep-copies and executes it for each
reference. Immutable non-foldable UDFs have the same problem. Please restrict
this exception to expressions proven cheap/pure and compile-time-backed, and
retain materialization for runtime-only constants.
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/CTEInlineTest.java:
##########
@@ -119,4 +128,69 @@ public void
refreshCteConsumersAfterNormalizeEliminatesEmptyBranch() {
connectContext.getSessionVariable().setDisableNereidsRules("");
}
}
+
+ @Test
+ public void testConstantOneRowCteAlwaysInlined() {
+ String sql = "WITH c AS (SELECT 1 AS a, 'x' AS b) "
+ + "SELECT * FROM c c1 JOIN c c2 ON c1.a = c2.a";
+
+ LogicalPlan analyzed = (LogicalPlan) PlanChecker.from(connectContext)
+ .analyze(sql)
+ .applyCustom(new PullUpCteAnchor())
+ .applyCustom(new CTEInline())
+ .getPlan();
+
+ assertNoCteNodes(analyzed);
+ }
+
+ @Test
+ public void testConstantOneRowCteWithManyConsumersInlined() {
+ String sql = "WITH consts AS ("
+ + " SELECT '2026-01-01' AS day_start, '2026-08-17' AS
day_end, "
+ + " '2025-01-01' AS tq_start, '2025-08-17' AS tq_end)"
+ + "SELECT * FROM consts c1, consts c2, consts c3, consts c4, "
+ + " consts c5, consts c6, consts c7, consts c8";
+
+ LogicalPlan analyzed = (LogicalPlan) PlanChecker.from(connectContext)
+ .analyze(sql)
+ .applyCustom(new PullUpCteAnchor())
+ .applyCustom(new CTEInline())
+ .getPlan();
+
+ assertNoCteNodes(analyzed);
+ }
+
+ @Test
+ public void testTableBackedCteNotForcedInline() {
+ String sql = "WITH t AS (SELECT id, score FROM T1) "
+ + "SELECT * FROM t x JOIN t y ON x.id = y.id";
+
+ PlanChecker.from(connectContext)
+ .analyze(sql)
+ .applyCustom(new PullUpCteAnchor())
+ .applyCustom(new CTEInline())
+ .matches(logicalCTEAnchor());
+ }
+
+ @Test
+ public void testNonDeterministicOneRowCteNotForcedInline() {
+ String sql = "WITH r AS (SELECT RANDOM() AS x) "
+ + "SELECT * FROM r r1 JOIN r r2 ON r1.x = r2.x";
+
+ PlanChecker.from(connectContext)
+ .analyze(sql)
+ .applyCustom(new PullUpCteAnchor())
+ .applyCustom(new CTEInline())
+ .matches(logicalCTEAnchor());
+ }
+
+ private static void assertNoCteNodes(LogicalPlan plan) {
+ plan.foreach(p -> {
+ if (p instanceof LogicalCTEAnchor || p instanceof
LogicalCTEProducer) {
Review Comment:
[P2] Also reject leftover CTE consumers
Inlining removes the anchor separately from replacing each
`LogicalCTEConsumer`. If the consumer traversal or CTE-id match regresses, the
plan can contain orphan consumers while this helper still passes because it
checks only anchors and producers. Include `LogicalCTEConsumer` in this
assertion so these tests actually prove that all logical CTE artifacts were
eliminated.
--
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]
