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

> 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