Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails - Mailing list pgsql-hackers

From Markus Winand
Subject Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails
Date
Msg-id 648B0505-AA57-42C2-A2DA-E551DE46FA15@winand.at
Whole thread Raw
In response to Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails  (torikoshia <torikoshia@oss.nttdata.com>)
Responses Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails  (Peter Eisentraut <peter.eisentraut@enterprisedb.com>)
List pgsql-hackers
Hi!

It seems like this patch causes another problem.

If I explain a simple row generator **without** verbose, it fails:

postgres=# EXPLAIN (VERBOSE FALSE)
           WITH RECURSIVE gen (n)  AS (
                  VALUES (1)
           UNION ALL
                  SELECT n+1
                    FROM gen
                   WHERE n < 3
           )
           SELECT * FROM gen
           ;
ERROR:  could not find RecursiveUnion for WorkTableScan with wtParam 0

That’s the new error message introduced by the patch.

The same with verbose works just fine:

postgres=# EXPLAIN (VERBOSE TRUE)
           WITH RECURSIVE gen (n)  AS (
                  VALUES (1)
           UNION ALL
                  SELECT n+1
                    FROM gen
                   WHERE n < 3
           )
           SELECT * FROM gen
           ;
                                 QUERY PLAN
-----------------------------------------------------------------------------
 CTE Scan on gen  (cost=2.95..3.57 rows=31 width=4)
   Output: gen.n
   CTE gen
     ->  Recursive Union  (cost=0.00..2.95 rows=31 width=4)
           ->  Result  (cost=0.00..0.01 rows=1 width=4)
                 Output: 1
           ->  WorkTable Scan on gen gen_1  (cost=0.00..0.23 rows=3 width=4)
                 Output: (gen_1.n + 1)
                 Filter: (gen_1.n < 3)
(9 rows)

Both variants work fine before that patch (4ac0f450b698442c3273ddfe8eed0e1a7e56645f).

Markus Winand
winand.at

> On 21.09.2021, at 14:43, torikoshia <torikoshia@oss.nttdata.com> wrote:
>
> On 2021-09-16 08:40, Tom Lane wrote:
>> I wrote:
>>> I do not think that patch is a proper solution, but we do need to do
>>> something about this.
>> I poked into this and decided it's an ancient omission within ruleutils.c.
>> The reason we've not seen it before is probably that you can't get to the
>> case through the parser.  The SEARCH stuff is generating a query structure
>> basically equivalent to
>> regression=# with recursive cte (x,r) as (
>> select 42 as x, row(i, 2.3) as r from generate_series(1,3) i
>> union all
>> select x, row((c.r).f1, 4.5) from cte c
>> )
>> select * from cte;
>> ERROR:  record type has not been registered
>> and as you can see, expandRecordVariable fails to figure out what
>> the referent of "c.r" is.  I think that could be fixed (by looking
>> into the non-recursive term), but given the lack of field demand,
>> I'm not feeling that it's urgent.
>> So the omission is pretty obvious from the misleading comment:
>> actually, Vars referencing RTE_CTE RTEs can also appear in WorkTableScan
>> nodes, and we're not doing anything to support that.  But we only reach
>> this code when trying to resolve a field of a Var of RECORD type, which
>> is a case that it seems like the parser can't produce.
>> It doesn't look too hard to fix: we just have to find the RecursiveUnion
>> that goes with the WorkTableScan, and drill down into that, much as we
>> would do in the CteScan case.  See attached draft patch.  I'm too tired
>> to beat on this heavily or add a test case, but I have verified that it
>> passes check-world and handles the example presented in this thread.
>>             regards, tom lane
>
> Thanks for looking into this and fixing it!
>
> --
> Regards,
>
> --
> Atsushi Torikoshi
> NTT DATA CORPORATION
>
>
>
>




pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Reword docs of feature "Remove temporary files after backend crash"
Next
From: Bharath Rupireddy
Date:
Subject: Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()