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

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