Thread: EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

From
"Lepikhov Andrei"
Date:
Hi,

While designing a CustomScan node, I got stuck into two errors:
1. "failed to find plan for CTE."
2. "failed to find plan for subquery."
After a short research, I found commit 3f50b82, which shows the problem's origins - setrefs don't change the varno of
custom_scan_tlistand can directly reference CTE or Subquery entry. In the "EXPLAIN VERBOSE" case, the deparsing routine
can'tfind dpns->inner_plan for such an entry.
 

Regards,
Andrei Lepikhov



Re: EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

From
Richard Guo
Date:

On Tue, Sep 5, 2023 at 3:23 PM Lepikhov Andrei <lepikhov@fastmail.com> wrote:
Hi,

While designing a CustomScan node, I got stuck into two errors:
1. "failed to find plan for CTE."
2. "failed to find plan for subquery."
After a short research, I found commit 3f50b82, which shows the problem's origins - setrefs don't change the varno of custom_scan_tlist and can directly reference CTE or Subquery entry. In the "EXPLAIN VERBOSE" case, the deparsing routine can't find dpns->inner_plan for such an entry.

I was able to reproduce both errors with the help of the query in [1]
and the extension provided in [2].  It seems that the assumption in the
case of RTE_SUBQUERY and RTE_CTE in get_name_for_var_field() does not
always hold:

 * the only place we'd see a Var directly referencing a
 * SUBQUERY RTE is in a SubqueryScan plan node

 * the only places we'd see a Var directly
 * referencing a CTE RTE are in CteScan or WorkTableScan
 * plan nodes.

But this issue shows that in a CustomScan node we can also see a Var
directly referencing a SUBQUERY RTE or CTE RTE.  (I suspect that it also
happens with ForeignScan node.)

So it seems that we need to assign a proper INNER referent for
CustomScan node in set_deparse_plan().  I tried 'trick.diff' in [1]
which uses linitial(dpns->subplans), it fixes the query there but would
crash the query below.

explain (verbose, costs off)
select (rr).column2 from
  (select r from (values(1,2),(3,4)) r) s join
  (select rr from (values(1,7),(3,8)) rr limit 2) ss
  on (r).column1 = (rr).column1;
server closed the connection unexpectedly

Maybe we can use the first plan in CustomScan->custom_plans as the INNER
referent?  I'm not sure.

--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -5004,6 +5004,13 @@ set_deparse_plan(deparse_namespace *dpns, Plan *plan)
    else if (IsA(plan, WorkTableScan))
        dpns->inner_plan = find_recursive_union(dpns,
                                                (WorkTableScan *) plan);
+   else if (IsA(plan, CustomScan))
+   {
+       CustomScan *cplan = (CustomScan *) plan;
+
+       if (cplan->custom_plans)
+           dpns->inner_plan = linitial(cplan->custom_plans);
+   }

Hi Tom, have you got a chance to look into this issue?

[1] https://www.postgresql.org/message-id/3f7bcdb7-c263-4c06-a138-140f5c3898ed%40app.fastmail.com
[2] https://www.postgresql.org/message-id/3933834e-b657-4ad1-bf4e-5f3fbba7ba14%40app.fastmail.com

Thanks
Richard

Re: EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

From
"Lepikhov Andrei"
Date:

On Mon, Sep 11, 2023, at 1:28 PM, Richard Guo wrote:
> On Tue, Sep 5, 2023 at 3:23 PM Lepikhov Andrei <lepikhov@fastmail.com> wrote:
>> Hi,
>>
>> While designing a CustomScan node, I got stuck into two errors:
>> 1. "failed to find plan for CTE."
>> 2. "failed to find plan for subquery."
>> After a short research, I found commit 3f50b82, which shows the problem's origins - setrefs don't change the varno
ofcustom_scan_tlist and can directly reference CTE or Subquery entry. In the "EXPLAIN VERBOSE" case, the deparsing
routinecan't find dpns->inner_plan for such an entry. 
>
> I was able to reproduce both errors with the help of the query in [1]
> and the extension provided in [2].  It seems that the assumption in the
> case of RTE_SUBQUERY and RTE_CTE in get_name_for_var_field() does not
> always hold:
>
>  * the only place we'd see a Var directly referencing a
>  * SUBQUERY RTE is in a SubqueryScan plan node
>
>  * the only places we'd see a Var directly
>  * referencing a CTE RTE are in CteScan or WorkTableScan
>  * plan nodes.

I have written the letter with second thoughts, because the logic of building CustomScan target list isn't clear for
me.Maybe I just have made a mistake in my code? 

>
> But this issue shows that in a CustomScan node we can also see a Var
> directly referencing a SUBQUERY RTE or CTE RTE.  (I suspect that it also
> happens with ForeignScan node.)

Maybe, but I couldn't imagine such a situation.

>
> So it seems that we need to assign a proper INNER referent for
> CustomScan node in set_deparse_plan().  I tried 'trick.diff' in [1]
> which uses linitial(dpns->subplans), it fixes the query there but would
> crash the query below.

I see here two different cases. Direct link to a Subquery need rte->subquery, which was nullified due to optimization.
Thecase with reference to CTE is more complex. which subplan of the statement subplans should we refer here? But it is
myfirst glance into this code, maybe someone understand it better. 

--
Regards,
Andrei Lepikhov



Re: EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

From
"Lepikhov Andrei"
Date:

On Mon, Sep 11, 2023, at 1:28 PM, Richard Guo wrote:
> Maybe we can use the first plan in CustomScan->custom_plans as the INNER
> referent?  I'm not sure.
After some exploration of ruleutils.c I grasped more on this issue.
If we have come to the CTE or SubPlan entry by the INDEX_VAR link from custom_scan_tlist, we don't have any information
onwhich subplan to go further (entry info already removed because of optimization).
 
To find a specific subplan, we must pass through the custom node subtree and find CteScan or SubqueryScan, whose target
listrefers to this entry. After that, we can go into the subplan to correctly explain the var.
 
It looks like a lot of work. One vague idea is to change set_customscan_references and wrap up CustomScan with useful
data.
Or, maybe it is better not to try to go under the custom_scan_tlist and explain CTE and Subquery right on the level of
theCustomScan?
 

-- 
Regards,
Andrei Lepikhov



Re: EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

From
Andrey Lepikhov
Date:
On 11/9/2023 13:28, Richard Guo wrote:
> So it seems that we need to assign a proper INNER referent for
> CustomScan node in set_deparse_plan().  I tried 'trick.diff' in [1]
> which uses linitial(dpns->subplans), it fixes the query there but would
> crash the query below.
> 
> explain (verbose, costs off)
> select (rr).column2 from
>    (select r from (values(1,2),(3,4)) r) s join
>    (select rr from (values(1,7),(3,8)) rr limit 2) ss
>    on (r).column1 = (rr).column1;
> server closed the connection unexpectedly
> 
> Maybe we can use the first plan in CustomScan->custom_plans as the INNER
> referent?  I'm not sure.

Looking into this issue more, I figured out that we can't be sure from 
which side the current varno of custom_scan_tlist was produced: from 
inner, outer or one of the custom subplans. In one of my previous R&D 
projects, I remember that the CustomScan node received tuples from the 
network, gathering them from other pg-instances.
So, as we already have an optional ExplainCustomScan routine, maybe add 
one more for the show_plan_tlist routine? It can print a target list or 
ignore it as the Append node does.

-- 
regards,
Andrey Lepikhov
Postgres Professional




Re: EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

From
Andrei Lepikhov
Date:
After some attempts to resolve this issue, I must admit that we should 
add a non-trivial method in the CustomScan node interface, which would 
be called in the ruleutils code.
I guess it is too much for such a rare situation. So maybe just add a 
documentation entry on this thing and set a couple of assertions on 
early detection of problems?.
In the attachment - is an offer on how to change the code.

-- 
regards,
Andrey Lepikhov
Postgres Professional

Attachment
On 09.10.2023 11:41, Andrei Lepikhov wrote:
After some attempts to resolve this issue, I must admit that we should add a non-trivial method in the CustomScan node interface, which would be called in the ruleutils code.
I guess it is too much for such a rare situation. So maybe just add a documentation entry on this thing and set a couple of assertions on early detection of problems?.
In the attachment - is an offer on how to change the code.

Hi!

I have studied the problem and I agree with your suggestion to disable the possibility of explaining subqueries in CustomNode for now.

So far I can only find information about HashJoin and ValueScan, but I expected that this is not what is needed.(gdb) p *((Plan *)(cscan->custom_plans->elements[0].ptr_value))->lefttree
$29 = {type = T_ValuesScan, startup_cost = 0, total_cost = 0.025000000000000001, plan_rows = 2, plan_width = 4, parallel_aware = false, parallel_safe = true, async_capable = false, plan_node_id = 0,
  targetlist = 0x560fc9979398, qual = 0x0, lefttree = 0x0, righttree = 0x0, initPlan = 0x0, extParam = 0x0, allParam = 0x0}
(gdb) p *((Plan *)(cscan->custom_plans->elements[0].ptr_value))
$30 = {type = T_HashJoin, startup_cost = 0.050000000000000003, total_cost = 0.10250000000000001, plan_rows = 2, plan_width = 32, parallel_aware = false, parallel_safe = false, async_capable = false,
  plan_node_id = 0, targetlist = 0x560fc9979208, qual = 0x0, lefttree = 0x560fc9979438, righttree = 0x560fc9979958, initPlan = 0x0, extParam = 0x0, allParam = 0x0}
(gdb) p *((Plan *)(cscan->custom_plans->elements[0].ptr_value))->righttree
$31 = {type = T_Hash, startup_cost = 0.025000000000000001, total_cost = 0.025000000000000001, plan_rows = 2, plan_width = 32, parallel_aware = false, parallel_safe = false, async_capable = false,
  plan_node_id = 0, targetlist = 0x560fc9979518, qual = 0x0, lefttree = 0x560fc9979698, righttree = 0x0, initPlan = 0x0, extParam = 0x0, allParam = 0x0}
(gdb) p *((Plan *)(cscan->custom_plans->elements[0].ptr_value))->lefttree
$32 = {type = T_ValuesScan, startup_cost = 0, total_cost = 0.025000000000000001, plan_rows = 2, plan_width = 4, parallel_aware = false, parallel_safe = true, async_capable = false, plan_node_id = 0,
  targetlist = 0x560fc9979398, qual = 0x0, lefttree = 0x0, righttree = 0x0, initPlan = 0x0, extParam = 0x0, allParam = 0x0}