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

David Geier <david@swarm64.com>

From: David Geier <david@swarm64.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Date: 2021-01-19T07:41:20Z
Lists: pgsql-hackers
Hi,

On 18.01.21 23:42, Tom Lane wrote:
> David Geier<david@swarm64.com>  writes:
>> On 18.01.21 19:46, Tom Lane wrote:
>>> 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.
> OK, cool.  I was afraid you'd argue that you really needed your CustomScan
> node to be transparent in such cases.  We could imagine inventing an
> additional custom-scan-provider callback to embed the necessary knowledge,
> but I'd rather not add the complexity until someone has a use-case.

I was thinking about that. Generally, having such possibility would be 
very useful. Unfortunately, I believe that in my specific case even 
having such functionality wouldn't allow for the query to work with 
CURRENT OF, because my CSP behaves similarly to a materialize node.

My understanding is only such plans are supported which consist of nodes 
that guarantee that the tuple returned by the plan is the unmodified 
tuple a scan leaf node is currently positioned on?

Still, if there's interest I would be happy to draft a patch. Instead of 
a separate CSP callback, we could also provide an additional flag like 
CUSTOMPATH_SUPPORT_CURRENT_OF. The advantage of the callback would be 
that we could delay the decision until execution time where potentially 
more information is available.
>> I updated the patch to match your proposal.
> WFM, will push in a bit.
>
> 			regards, tom lane
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.