Hi Andrey, Thanks for the clarification. That explains the difference I saw: the CTE itself had already been inlined by SS_process_ctes, while the remaining cteList still prevented the surrounding subquery from being pulled up.
I tested v2 on current master. For the inlineable cases, both a NOT MATERIALIZED CTE and a default singly-referenced CTE were successfully pulled up. In both cases the planner produced a nested-loop plan using the primary-key index on the inner table through the outer join condition. I also retested the negative cases: * A MATERIALIZED CTE still prevents pull-up and remains a CTE Scan. * A CTE containing a volatile function still remains a CTE Scan. * A default multiply-referenced CTE remains materialized and is read through two CTE Scan nodes. * I also tested an unreferenced SELECT CTE specifically for the cterefcount == 0 case. The surrounding subquery remains a Subquery Scan, so it is not incorrectly accepted through the new all-inlineable path. For the representative inlineable case, I also compared the query result with the equivalent query without the CTE, and the results were identical. The updated SS_all_ctes_inlineable() comment now matches the implementation. I did not find any new issues in these tests. I couldn't find this patch in the CommitFest app. If you plan to submit it for the next CommitFest, I'd be happy to add myself as a reviewer there as well. Regards, Clemenza On Tue, Sep 8, 2026 at 10:19 PM Andrey Kazarinov <[email protected]> wrote: > > > I could not reproduce this exact before-patch plan on current master. > > With the NOT MATERIALIZED example, current unpatched master already > > produces a direct Seq Scan on cte_pullup_t rather than a CTE Scan on > > cte. > > > > Hash Right Join > > Hash Cond: (cte_pullup_t.id = s.tid) > > -> Seq Scan on cte_pullup_t > > -> Hash > > -> Seq Scan on cte_pullup_s s > > Filter: (id < 5) > > > > The patch still changes the higher-level plan shape in my test: the > > inlineable-CTE case becomes the same general shape as the equivalent > > no-CTE query. > > Hi Clemenza, > > Thank you for testing and for the detailed feedback. > > Here is a reproducible example using the standard regression tables > tenk1 and tenk2 (both have 10000 rows; tenk1.unique1 is a primary key): > > explain (costs off) > select * from tenk2 s left join ( > with cte as not materialized (select unique1, two from tenk1) > select * from (select unique1, two from cte) sub > ) t on t.unique1 = s.unique1 > where s.unique1 < 10; > > Before the patch (unpatched master): > Hash Right Join > Hash Cond: (tenk1.unique1 = s.unique1) > -> Seq Scan on tenk1 > -> Hash > -> Bitmap Heap Scan on tenk2 s > Recheck Cond: (unique1 < 10) > -> Bitmap Index Scan on tenk2_unique1 > Index Cond: (unique1 < 10) > > After the patch: > Nested Loop Left Join > -> Seq Scan on tenk2 s > Filter: (unique1 < 10) > -> Index Scan using tenk1_pkey on tenk1 > Index Cond: (unique1 = s.unique1) > > Without the patch, the subquery is planned separately: SS_process_ctes > inlines the CTE, but is_simple_subquery still rejects the subquery > because cteList is non-empty - the planner cannot see that the join > condition t.unique1 = s.unique1 could use the primary key index on > tenk1, > so it falls back to a hash join with a full seq scan. > > With the patch, the subquery is pulled up into the parent query. > The planner can now see through the former subquery boundary and chooses > a > nested loop with index scan (only 10 index lookups instead of scanning > 10000 rows). > > > > I also noticed a small inconsistency in the comment above > > SS_all_ctes_inlineable(). It says that every CTE is either > > "unreferenced (SELECT) or passes the inlineability checks", but the > > implementation explicitly returns false for: > > ``` > > if (cte->cterefcount == 0 && cmdType == CMD_SELECT) > > return false; > > ``` > > This behavior matches the explanation in your email, so I think the > > comment may just need adjustment. > > > > Regards, > > Clemenza Zhang > > I have fixed the comment above SS_all_ctes_inlineable(). > The function header comment now briefly notes that unreferenced CTEs > cause > it to return false, and the inline comment at the check site explains > the reason in detail: unreferenced SELECT CTEs (cterefcount == 0) are > neither inlined nor materialized by SS_process_ctes -- they are simply > skipped with a dummy entry in cte_plan_ids. > > Updated patch attached. > > P.S. I most likely continue discussion from another email: > [email protected] > > Regards, > Andrey Kazarinov
