Re: [PATCH] Allow subquery pull-up past inlineable CTEs

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
<a(dot)kazarinov(at)postgrespro(dot)ru> 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:
> kazarandrey(at)yandex(dot)ru
>
> Regards,
> Andrey Kazarinov