EXPLAIN Verbose issue - custom_scan_tlist can directly refer CTE and Subquery

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]/messages/by-id/3f7bcdb7-c263-4c06-a138-140f5c3898ed@app.fastmail.com
and the extension provided in [2]/messages/by-id/3933834e-b657-4ad1-bf4e-5f3fbba7ba14@app.fastmail.com. 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]/messages/by-id/3f7bcdb7-c263-4c06-a138-140f5c3898ed@app.fastmail.com
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]: /messages/by-id/3f7bcdb7-c263-4c06-a138-140f5c3898ed@app.fastmail.com
/messages/by-id/3f7bcdb7-c263-4c06-a138-140f5c3898ed@app.fastmail.com
[2]: /messages/by-id/3933834e-b657-4ad1-bf4e-5f3fbba7ba14@app.fastmail.com
/messages/by-id/3933834e-b657-4ad1-bf4e-5f3fbba7ba14@app.fastmail.com

Thanks
Richard

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 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.

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. The case with reference to CTE is more complex. which subplan of the statement subplans should we refer here? But it is my first glance into this code, maybe someone understand it better.

--
Regards,
Andrei Lepikhov

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 on which 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 list refers 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 the CustomScan?

--
Regards,
Andrei Lepikhov

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

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

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}