Horton

Re: search_plan_tree(): handling of non-leaf CustomScanState nodes causes segfault

David Geier <david@swarm64.com>

View thread View on postgr.es
From: David Geier <david@swarm64.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Date: 2021-01-18T21:32:12Z
Lists: pgsql-hackers

Attachments

Hi,

On 18.01.21 19:46, Tom Lane wrote:
> David Geier <david@swarm64.com> writes:
>> search_plan_tree() assumes that
>> CustomScanState::ScanState::ss_currentRelation is never NULL. In my
>> understanding that only holds for CustomScanState nodes which are at the
>> bottom of the plan and actually read from a relation. CustomScanState
>> nodes which are not at the bottom don't have ss_currentRelation set. I
>> believe for such nodes, instead search_plan_tree() should recurse into
>> CustomScanState::custom_ps.
> Hm.  I agree that we shouldn't simply assume that ss_currentRelation
> isn't null.  However, we cannot make search_plan_tree() descend
> through non-leaf CustomScan nodes, because we don't know what processing
> is involved there.  We need to find a scan that is guaranteed to return
> rows that are one-to-one with the cursor output.  This is why the function
> doesn't descend through join or aggregation nodes, and I see no argument
> by which we should assume we know more about what a customscan node will
> do than we know about those.
That makes sense. Thanks for the explanation.
>
> So I'm inclined to think a suitable fix is just
>
> -               if (RelationGetRelid(sstate->ss_currentRelation) == table_oid)
> +               if (sstate->ss_currentRelation &&
> +                   RelationGetRelid(sstate->ss_currentRelation) == table_oid)
>                      result = sstate;
>
> 			regards, tom lane
>
>
I updated the patch to match your proposal.

Best regards,
David
Swarm64

Commits

  1. Remove faulty support for MergeAppend plan with WHERE CURRENT OF.

  2. Avoid crash with WHERE CURRENT OF and a custom scan plan.